[Mesa-dev] [PATCH 09/12] Android: push driver build details to driver makefiles

[Emil Velikov]

On 2 May 2017 at 17:06, Rob Herring <robh at kernel.org> wrote:
> On Tue, May 2, 2017 at 10:14 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 2 May 2017 at 15:41, Rob Herring <robh at kernel.org> wrote:
>>> On Mon, May 1, 2017 at 2:18 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> Hi Rob,
>>>>
>>>> On 27 April 2017 at 20:43, Rob Herring <robh at kernel.org> wrote:
>>>
>>> [...]
>>>
>>>>> 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.
>>>
>>> To ensure GALLIUM_*LIBS is filled out by all the drivers before
>>> targets/dri. The Android build system doesn't let us have 2 includes
>>> and the order returned by all-named-subdir-makefiles varies between
>>> make and kati. I'll add a comment.
>>>
>> Guess one really wants to fix make and kati to align, but for the
>> moment a workaround in Mesa should be fine.
>
> I think the fundamental problem is $(wildcard) is used which
> explicitly does not have any ordering guarantees. It may be possible
> to write the macro in terms of $(foreach) to preserve the order.
>
> OTOH, I think it is good to be explicit that we have this ordering dependency.
>
>>>>> 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?
>>>
>>> Otherwise we don't get the current value of LOCAL_* here as
>>> GALLIUM_*LIBS will get expanded later.
>>>
>> Indeed, thank you.
>>
>>>>> 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.
>>>
>>> The winsys is added by the drivers. This is just additional
>>> dependencies the winsys has.
>>>
>> So if I understand you correctly, Android does not link the
>> shared/static libraries if the object is a static library.
>> Thus one needs to track those again in the final link.
>
> Right. Though if you use the LOCAL_WHOLE_STATIC_LIBRARIES, then all
> the static libs are linked.
>
>> That's a bit counter intuitive since it means that despite using
>> "LIBRARIES" variable, Android only uses the headers.
>> Regardless...
>
> I guess that's right, but that's not really Android specific in that
> the final link needs all of the shared libraries.
>
Actually autoconf, cmake and scons can do that. That is:
Either build system can track the shared/static library requirements
of your intermediate static object. As such one does not need to
remember and add them again in the final link - it's done
automatically.

None of that really help us here, so let's just ignore it for now.

Thanks
Emil