Re: [CI] drm/i915: Move GraphicsTechnology files under gt/

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 24 Apr 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Start partitioning off the code that talks to the hardware (GT) from the
> uapi layers and move the device facing code under gt/
>
> One casualty is s/intel_ringbuffer.h/intel_engine.h/ with the plan to
> subdivide that header and body further (and split out the submission
> code from the ringbuffer and logical context handling). This patch aims
> to be simple motion so git can fixup inflight patches with little mess.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Yay! I guess this'll conflict badly with my wip header refactoring, but
first come first served.

A couple of comments below. I don't mind having them fixed afterwards to
avoid several CI rounds and potential conflicts on the huge patch now.

> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 53ff209b91bb..40130cf5c003 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -35,32 +35,53 @@ subdir-ccflags-y += \
>  # Extra header tests
>  include $(src)/Makefile.header-test
>  
> +subdir-ccflags-y += -I$(src)

Just remembered [1], the kbuild folks want to use -I $(srctree)/$(src).

I suppose it's by design you don't add gt/ in the header search path,
and require explicit "gt/foo.h" in the #includes in files outside the
gt/ directory. (I assume files under gt/ can omit the directory part.) I
could be persuaded either way.

[1] http://mid.mail-archive.com/1553859161-2628-1-git-send-email-yamada.masahiro@socionext.com

> +
>  # Please keep these build lists sorted!
>  
>  # core driver code
>  i915-y += i915_drv.o \
>  	  i915_irq.o \
> -	  i915_memcpy.o \
> -	  i915_mm.o \
>  	  i915_params.o \
>  	  i915_pci.o \
> -	  i915_reset.o \
>  	  i915_suspend.o \
> -	  i915_sw_fence.o \
> -	  i915_syncmap.o \
>  	  i915_sysfs.o \
> -	  i915_user_extensions.o \
>  	  intel_csr.o \
>  	  intel_device_info.o \
>  	  intel_pm.o \
>  	  intel_runtime_pm.o \
> -	  intel_workarounds.o
> +	  intel_uncore.o
> +
> +# core library code
> +i915-y += \
> +	i915_memcpy.o \
> +	i915_mm.o \
> +	i915_sw_fence.o \
> +	i915_syncmap.o \
> +	i915_user_extensions.o
>  
>  i915-$(CONFIG_COMPAT)   += i915_ioc32.o
>  i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>  i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>  
> -# GEM code
> +# "Graphics Technology" (aka we talk to the gpu)
> +obj-y += gt/

I wonder if this and the gt/Makefile are subtle enough to warrant a
comment. That they are only used for the header tests. It took me a
moment to figure this out.

> +gt-y += \
> +	gt/intel_breadcrumbs.o \
> +	gt/intel_context.o \
> +	gt/intel_engine_cs.o \
> +	gt/intel_hangcheck.o \
> +	gt/intel_lrc.o \
> +	gt/intel_reset.o \
> +	gt/intel_ringbuffer.o \
> +	gt/intel_mocs.o \
> +	gt/intel_sseu.o \
> +	gt/intel_workarounds.o
> +gt-$(CONFIG_DRM_I915_SELFTEST) += \
> +	gt/mock_engine.o
> +i915-y += $(gt-y)
> +
> +# GEM (Graphics Execution Management) code
>  i915-y += \
>  	  i915_active.o \
>  	  i915_cmd_parser.o \
> @@ -88,15 +109,6 @@ i915-y += \
>  	  i915_timeline.o \
>  	  i915_trace_points.o \
>  	  i915_vma.o \
> -	  intel_breadcrumbs.o \
> -	  intel_context.o \
> -	  intel_engine_cs.o \
> -	  intel_hangcheck.o \
> -	  intel_lrc.o \
> -	  intel_mocs.o \
> -	  intel_ringbuffer.o \
> -	  intel_sseu.o \
> -	  intel_uncore.o \
>  	  intel_wopcm.o
>  
>  # general-purpose microcontroller (GuC) support
> diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test
> index 5bcc78d7ac96..96a5d90629ec 100644
> --- a/drivers/gpu/drm/i915/Makefile.header-test
> +++ b/drivers/gpu/drm/i915/Makefile.header-test
> @@ -13,13 +13,11 @@ header_test := \
>  	intel_cdclk.h \
>  	intel_color.h \
>  	intel_connector.h \
> -	intel_context_types.h \
>  	intel_crt.h \
>  	intel_csr.h \
>  	intel_ddi.h \
>  	intel_dp.h \
>  	intel_dvo.h \
> -	intel_engine_types.h \
>  	intel_fbc.h \
>  	intel_fbdev.h \
>  	intel_frontbuffer.h \
> @@ -33,9 +31,7 @@ header_test := \
>  	intel_psr.h \
>  	intel_sdvo.h \
>  	intel_sprite.h \
> -	intel_sseu.h \
> -	intel_tv.h \
> -	intel_workarounds_types.h
> +	intel_tv.h
>  
>  quiet_cmd_header_test = HDRTEST $@
>        cmd_header_test = echo "\#include \"$(<F)\"" > $@
> diff --git a/drivers/gpu/drm/i915/gt/Makefile b/drivers/gpu/drm/i915/gt/Makefile
> new file mode 100644
> index 000000000000..1c75b5c9790c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/Makefile
> @@ -0,0 +1,2 @@
> +# Extra header tests
> +include $(src)/Makefile.header-test
> diff --git a/drivers/gpu/drm/i915/gt/Makefile.header-test b/drivers/gpu/drm/i915/gt/Makefile.header-test
> new file mode 100644
> index 000000000000..61e06cbb4b32
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/Makefile.header-test
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: MIT
> +# Copyright © 2019 Intel Corporation
> +
> +# Test the headers are compilable as standalone units
> +header_test := $(notdir $(wildcard $(src)/*.h))
> +
> +quiet_cmd_header_test = HDRTEST $@
> +      cmd_header_test = echo "\#include \"$(<F)\"" > $@
> +
> +header_test_%.c: %.h
> +	$(call cmd,header_test)
> +
> +extra-$(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)))


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx