Hi Rob,
On 27 April 2017 at 20:43, Rob Herring <r...@kernel.org> wrote:
> +classic_drivers := i915.HAVE_I915_DRI i965.HAVE_I965_DRI
> +gallium_drivers := \
> + swrast.HAVE_GALLIUM_SOFTPIPE \
> + freedreno.HAVE_GALLIUM_FREEDRENO \
> + i915g.HAVE_GALLIUM_I915 \
> + nouveau.HAVE_GALLIUM_NOUVEAU \
> + r300g.HAVE_GALLIUM_R300 \
> + r600g.HAVE_GALLIUM_R600 \
> + radeonsi.HAVE_GALLIUM_RADEONSI \
> + vmwgfx.HAVE_GALLIUM_VMWGFX \
> + vc4.HAVE_GALLIUM_VC4 \
> + virgl.HAVE_GALLIUM_VIRGL
>
> -MESA_GPU_DRIVERS := $(strip $(BOARD_GPU_DRIVERS))
> -
> -# warn about invalid drivers
> -invalid_drivers := $(filter-out \
> - $(classic_drivers) $(gallium_drivers), $(MESA_GPU_DRIVERS))
> -ifneq ($(invalid_drivers),)
> -$(warning invalid GPU drivers: $(invalid_drivers))
> -# tidy up
> -MESA_GPU_DRIVERS := $(filter-out $(invalid_drivers), $(MESA_GPU_DRIVERS))
> -endif
> +MESA_BUILD_CLASSIC := $(strip $(foreach d, $(BOARD_GPU_DRIVERS), $(patsubst
> $(d).%,%, $(filter $(d).%, $(classic_drivers)))))
> +MESA_BUILD_GALLIUM := $(strip $(foreach d, $(BOARD_GPU_DRIVERS), $(patsubst
> $(d).%,%, $(filter $(d).%, $(gallium_drivers)))))
> +$(foreach d, $(MESA_BUILD_CLASSIC) $(MESA_BUILD_GALLIUM), $(eval $(d) :=
> true))
>
Can you please add a one line description about the driver.HAVE_FOO pairs.
Be that here or above the declaration {classic,gallium}_drivers above.
> --- a/src/egl/Android.mk
> +++ b/src/egl/Android.mk
> @@ -57,16 +57,10 @@ LOCAL_SHARED_LIBRARIES := \
> libcutils \
> libsync
>
> -ifeq ($(strip $(MESA_BUILD_CLASSIC)),true)
> -# require i915_dri and/or i965_dri
> -LOCAL_REQUIRED_MODULES += \
> - $(addsuffix _dri, $(filter i915 i965, $(MESA_GPU_DRIVERS)))
> -endif # MESA_BUILD_CLASSIC
> -
> -ifeq ($(strip $(MESA_BUILD_GALLIUM)),true)
> -LOCAL_REQUIRED_MODULES += gallium_dri
> -endif # MESA_BUILD_GALLIUM
> -
> +# This controls enabling building of driver libraries
> +LOCAL_REQUIRED_MODULES += $(if $(HAVE_I915_DRI),i915_dri,)
> +LOCAL_REQUIRED_MODULES += $(if $(HAVE_I965_DRI),i965_dri,)
> +LOCAL_REQUIRED_MODULES += $(if $(MESA_BUILD_GALLIUM),gallium_dri,)
>
Can I interest you in expanding the ifs as originally? Having control
flow on a single line can be a bit confusing.
> LOCAL_MODULE := libGLES_mesa
> LOCAL_MODULE_RELATIVE_PATH := egl
> diff --git a/src/gallium/Android.mk b/src/gallium/Android.mk
> index 7c6bda68d59f..d591aaf62e6a 100644
> --- a/src/gallium/Android.mk
> +++ b/src/gallium/Android.mk
> @@ -42,7 +42,9 @@ SUBDIRS += winsys/amdgpu/drm winsys/radeon/drm
> SUBDIRS += winsys/vc4/drm drivers/vc4
> SUBDIRS += winsys/virgl/drm winsys/virgl/vtest drivers/virgl
> SUBDIRS += winsys/svga/drm drivers/svga
> +SUBDIRS += state_trackers/dri
>
> -SUBDIRS += state_trackers/dri targets/dri
> +INC_DIRS := $(call all-named-subdir-makefiles,$(SUBDIRS))
> +INC_DIRS += $(call all-named-subdir-makefiles,targets/dri)
>
> -include $(call all-named-subdir-makefiles,$(SUBDIRS))
> +include $(INC_DIRS)
Why do we need the extra INC_DIRS here? If required please add a small
comment or drop it otherwise.
> diff --git a/src/gallium/drivers/freedreno/Android.mk
> b/src/gallium/drivers/freedreno/Android.mk
> index 5c97d9ef2906..330e82420426 100644
> --- a/src/gallium/drivers/freedreno/Android.mk
> +++ b/src/gallium/drivers/freedreno/Android.mk
> @@ -48,3 +48,8 @@ LOCAL_MODULE := libmesa_pipe_freedreno
> include $(LOCAL_PATH)/Android.gen.mk
> include $(GALLIUM_COMMON_MK)
> include $(BUILD_STATIC_LIBRARY)
> +
> +ifneq ($(HAVE_GALLIUM_FREEDRENO),)
> +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_freedreno)
> +$(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
Silly moment - why do we need eval here and below?
> +ifneq ($(HAVE_GALLIUM_R600),)
> +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) $(LOCAL_STATIC_LIBRARIES) \
> + libmesa_winsys_radeon)
Nit: maybe write this as below. We're a bit inconsistent in mesa so
feel free to ignore.
$(eval GALLIUM_LIBS += \
$(LOCAL_MODULE) \
$(LOCAL_STATIC_LIBRARIES) \
libmesa_winsys_radeon)
> LOCAL_WHOLE_STATIC_LIBRARIES := \
> - $(gallium_DRIVERS) \
> + $(sort $(GALLIUM_LIBS)) \
Please add a small comment about sort.
We use sort with GALLIUM_LIBS and GALLIUM_SHARED_LIBS in order to
remove any duplicate libraries.
> --- a/src/gallium/winsys/amdgpu/drm/Android.mk
> +++ b/src/gallium/winsys/amdgpu/drm/Android.mk
> @@ -41,3 +41,8 @@ LOCAL_MODULE := libmesa_winsys_amdgpu
>
> include $(GALLIUM_COMMON_MK)
> include $(BUILD_STATIC_LIBRARY)
> +
> +ifneq ($(HAVE_GALLIUM_RADEONSI),)
> +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) $(LOCAL_STATIC_LIBRARIES))
> +$(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
> +endif
> diff --git a/src/gallium/winsys/i915/drm/Android.mk
> b/src/gallium/winsys/i915/drm/Android.mk
> index b38bd8dca06a..bab3e85c5dd0 100644
> --- a/src/gallium/winsys/i915/drm/Android.mk
> +++ b/src/gallium/winsys/i915/drm/Android.mk
> @@ -35,3 +35,7 @@ LOCAL_MODULE := libmesa_winsys_i915
>
> include $(GALLIUM_COMMON_MK)
> include $(BUILD_STATIC_LIBRARY)
> +
> +ifneq ($(HAVE_GALLIUM_I915),)
> +$(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
> +endif
Not 100% sure if we need the winsys hunks. Please add small comment
covering why.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
