Re: [PATCH] libdrm: intel/Android.mk: Filter libdrm_intel library requirements on x86

[John Stultz <john.stultz@linaro.org>]

On Wed, Jan 31, 2018 at 8:01 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 30 January 2018 at 05:42, John Stultz <john.stultz@linaro.org> wrote:
>> On Fri, Jan 26, 2018 at 10:33 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> Hi all,
>>>
>>> Couple of ideas/notes,
>>>
>>> On 10 January 2018 at 20:36, Rob Herring <robh@kernel.org> wrote:
>>>> On Wed, Jan 10, 2018 at 1:09 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>>> On Wed, Jan 10, 2018 at 5:48 AM, Rob Herring <robh@kernel.org> wrote:
>>>>>> On Tue, Jan 9, 2018 at 11:25 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>>>>> When building AOSP after updating libdrm project to the
>>>>>>> freedesktop/master branch, I've seen the following build errors:
>>>>>>>
>>>>>>> external/libdrm/intel/Android.mk: error: libdrm_intel
>>>>>>
>>>>>> This is only needed for i915 (not i965) now BTW. I'm not sure at what
>>>>>> point we stop caring about i915.
>>> If you're using any other Intel components - say Beignet or the va
>>> driver, I think those still use libdrm_intel.
>>>
>>> An alternative solution IMHO, is to drop/tweak the API to not bother
>>> libpciaccess.
>>> I have some ancient cleanup/rework branch
>>>
>>> https://github.com/evelikov/libdrm/commits/intel-remove-legacy
>>
>> I'm not opposed to this, but I'm really not familiar with intel use
>> cases and what would be ok or not there.
>>
>>
> The unfortunate part is that people familiar don't have to
> time/interest to weight in :-(
> I might give it another try, one of these days. Unless someone beats me to it.
>
>>>>>>> (SHARED_LIBRARIES android-arm64) missing libpciaccess
>>>>>>> (SHARED_LIBRARIES android-arm64) You can set
>>>>>>> ALLOW_MISSING_DEPENDENCIES=true in your environment if this is
>>>>>>> intentional, but that may defer real problems until later in the
>>>>>>> build.
>>>>>>>
>>>>>>> Using ALLOW_MISSING_DEPENDENCIES=true when building allows
>>>>>>> things to function properly, but is not ideal.
>>>>>>>
>>>>>>> So basically, while I'm not including the libdrm_intel package
>>>>>>> into the build, just the fact that the Android.mk file references
>>>>>>> libpciaccess which isn't a repo included in AOSP causes the build
>>>>>>> failure.
>>>>>>>
>>>>>>> So it seems we need some sort of conditional filter in the
>>>>>>> Android.mk to skip over it if we're not building for intel.
>>>>>>>
>>>>>>> This is my initial attempt at solving this.
>>>>>>>
>>>>>>> Feedback would be greatly appreciated!
>>>>>>>
>>>>>>> I note that in the AOSP version of libdrm, the reference to
>>>>>>> libpciaccess has been removed. See:
>>>>>>>  https://android.googlesource.com/platform/external/libdrm/+/f6a1130dffae8de9ddd0c379066daf1df27fc8af%5E%21/
>>>>>>> So I wonder if it make sense to instead remove this upstream as
>>>>>>> well?
>>>>>>
>>>>>> Only if we drop i915.
>>>>>
>>>>> To be more precise, drop i915 for Android builds (I'm not suggesting
>>>>> dropping it elsewhere, just for the Android.mk). I'm really not sure
>>>>> which devices might be affected. Anyone able to point me to someone in
>>>>> Intel who would know?
>>>>
>>>> The android-x86 folks would be the ones to ask. I added Chih-Wei.
>>>>
>>> A really silly question - how are you triggering any of this if you're
>>> building on !x86?
>>> Is that because the GPU driver is not selected thus you we fall-back
>>> to "build all"?
>>
>> I think its mostly due the fact we're using the toplevel Android.mk
>> which includes all Android.mk files in subdirectories.
>>
> That does not matter. Unless otherwise stated the objects are optional.
> Thus they should not be build, unless...
>
> Android changed the policy or you're somehow building stuff that's not required?

I don't think its a policy, its seems its just how the toplevel
Android.mk file is setup:
   https://cgit.freedesktop.org/drm/libdrm/tree/Android.mk#n63

Where it includes all the Android.mk from all subdirectories, which
pulls in the intel/Android.mk, which adds libpciaccess to the
LOCAL_SHARED_LIBRARIES
   https://cgit.freedesktop.org/drm/libdrm/tree/intel/Android.mk#n36


>> I'm not sure quite how to select a gpu driver with the Android build
>> system, other then specifying it via a build variable, which is in
>> effect what I'm trying to do with this patch.
>>
>> Other ideas?
>>
> Based on your input seems like it's not set (grep for
> BOARD_GPU_DRIVERS), which results in "everything" being build.
> Thus optional libraries (like libdrm_intel) are pulled causing the problem.
>
> My earlier suggestion doesn't sound too crazy, although I'd check with
> RobH and the Android-x86 people.
> It might require one line change to the device manifest ;-)

So I looked a bit at this, but it seems that just controls which
components gets added from the libkms dir, not the top level
directories.

But we could add similar logic to the top level Android.mk file, using
the same BOARD_GPU_DRIVERS value. Hopefully that wouldn't break folks.

Since I'm not sure what else would be a better idea, I'll take a spin
at that, but if you have thoughts on it please do let me know.

Thanks so much for the feedback!
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel