On 1 February 2018 at 14:59, Rob Herring <robh@xxxxxxxxxx> wrote: > On Wed, Jan 31, 2018 at 12:46 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote: >> On Wed, Jan 31, 2018 at 8:01 AM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote: >>> On 30 January 2018 at 05:42, John Stultz <john.stultz@xxxxxxxxxx> wrote: >>>> On Fri, Jan 26, 2018 at 10:33 AM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote: >>>>> Hi all, >>>>> >>>>> Couple of ideas/notes, >>>>> >>>>> On 10 January 2018 at 20:36, Rob Herring <robh@xxxxxxxxxx> wrote: >>>>>> On Wed, Jan 10, 2018 at 1:09 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote: >>>>>>> On Wed, Jan 10, 2018 at 5:48 AM, Rob Herring <robh@xxxxxxxxxx> wrote: >>>>>>>> On Tue, Jan 9, 2018 at 11:25 PM, John Stultz <john.stultz@xxxxxxxxxx> 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 > > That's not quite right. The build system descends directories til it > finds an Android.mk. For subdirs under that Android.mk, the Android.mk > file must descend. That's why we have: > > include $(call all-makefiles-under,$(LOCAL_PATH)) > AFAICT the "include" has nothing to do with what gets build (by default). This page [1] states that, LOCAL_MODULE_TAGS - it's not set, defaults to optional. And anything optional is not build/installed. [1] https://source.android.com/setup/add-device >>>> 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. >> Please grep through the whole tree - namely device manifests, Mesa, libdrm and gralloc(s). With my proposed solution: - device manifest - say, "x86-generic", "arm-generic" "nouveau", "x86-only" [etc. but no "all"] sets BOARD_GPU_DRIVERS - mesa adds the respective dri module(s) to LOCAL_REQUIRED_MODULES - drm/gbm/other gralloc does pretty much the same thing With the correct (until sorted otherwise) gralloc is pulled from the device manifest >> 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. > > I prefer to not expand the use of BOARD_GPU_DRIVERS and leave it to > dependencies setting what to build. > That also works. For example the "nouveau" case pulls nouveau_dri.so and libdrm_nouveau.so Advantages of the BOARD_GPU_DRIVERS are that: - set once, don't care about gralloc/mesa/libdrm/other specifics - can have meta groups - the above generic/only ones >> 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. > > I think we've beat this one to death. Just fix the TARGET_ARCH check > to cover x86 and x86_64. The filter or filter-out make function should > work for this. > Pardon for being a pest, but can someone explain why/how say libdrm_intel gets pulled? Is that because BOARD_GPU_DRIVERS is not set, thus Mesa falls back to "all dri modules" and that attempts to build i915/i965_dri.so. With the tatter of which pulling libdrm_intel? If that's the case, special casing on ARCH is not good, since foo_dri.so may end up without libdrm_foo. Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
