| Message ID | 1515561939-28990-1-git-send-email-john.stultz@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libdrm: intel/Android.mk: Filter libdrm_intel library requirements on x86 | expand |
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. > (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. > > While this patch addresses upstream's Andorid.mk, I also notice > that the AOSP version of libdrm has converted to Android.bp files: > https://android.googlesource.com/platform/external/libdrm/+/fa32e29a1fe81e5472aabc65d3aa25a5af5aec55%5E%21/ > and wonder if getting that conversion upstream would be a good > idea here? No, because we still support Android versions back to L. There's also some desire to get meson builds to work in Android build system so we don't have to support multiple build systems. > > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Cc: Chad Versace <chad.versace@linux.intel.com> > Cc: Marissa Wall <marissaw@google.com> > Cc: Sean Paul <seanpaul@google.com> > Cc: Rob Herring <rob.herring@linaro.org> > Cc: Dan Willemsen <dwillemsen@google.com> > Cc: Tomasz Figa <tfiga@google.com> > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > intel/Android.mk | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/intel/Android.mk b/intel/Android.mk > index 5407ff3..d834692 100644 > --- a/intel/Android.mk > +++ b/intel/Android.mk > @@ -21,6 +21,7 @@ > # IN THE SOFTWARE. > # > > +ifeq ($(TARGET_ARCH), x86) This doesn't work for x86_64. i915 and 64-bit may not be a valid combination, not sure, but we do at least build test that.
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. > >> (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? >> +ifeq ($(TARGET_ARCH), x86) > > This doesn't work for x86_64. i915 and 64-bit may not be a valid > combination, not sure, but we do at least build test that. Out of curiosity, which environment is being used for this build testing? Are you describing your generic-build/qemu work or something else done as part of freedesktop.org? thanks -john
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. >> >>> (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. >>> +ifeq ($(TARGET_ARCH), x86) >> >> This doesn't work for x86_64. i915 and 64-bit may not be a valid >> combination, not sure, but we do at least build test that. > > Out of curiosity, which environment is being used for this build > testing? Are you describing your generic-build/qemu work or something > else done as part of freedesktop.org? The CI job I setup to build mesa master (and libdrm implicitly). Rob
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 >>> >>>> (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"? If so, it might be better to change things to: - error out if none selected - allow one to select "all", "x86", "arm" and similar groups thus only the things that can build are build eg. RobH had fun with x86 intrinsics while building the intel Vulkan driver on ARM -Emil
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. >>>>> (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. > If so, it might be better to change things to: > - error out if none selected > - allow one to select "all", "x86", "arm" and similar groups thus > only the things that can build are build > eg. RobH had fun with x86 intrinsics while building the intel Vulkan > driver on ARM 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? Thanks so much for the feedback! -john
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? >> If so, it might be better to change things to: >> - error out if none selected >> - allow one to select "all", "x86", "arm" and similar groups thus >> only the things that can build are build >> eg. RobH had fun with x86 intrinsics while building the intel Vulkan >> driver on ARM > > 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 ;-) Thanks Emil
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
On Wed, Jan 31, 2018 at 12:46 PM, John Stultz <john.stultz@linaro.org> wrote: > 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 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)) >>> 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. I prefer to not expand the use of BOARD_GPU_DRIVERS and leave it to dependencies setting what to build. > 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. Rob
On 1 February 2018 at 14:59, Rob Herring <robh@kernel.org> wrote: > On Wed, Jan 31, 2018 at 12:46 PM, John Stultz <john.stultz@linaro.org> wrote: >> 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 > > 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
diff --git a/intel/Android.mk b/intel/Android.mk index 5407ff3..d834692 100644 --- a/intel/Android.mk +++ b/intel/Android.mk @@ -21,6 +21,7 @@ # IN THE SOFTWARE. # +ifeq ($(TARGET_ARCH), x86) LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) @@ -37,3 +38,4 @@ LOCAL_SHARED_LIBRARIES := \ include $(LIBDRM_COMMON_MK) include $(BUILD_SHARED_LIBRARY) +endif
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 (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? While this patch addresses upstream's Andorid.mk, I also notice that the AOSP version of libdrm has converted to Android.bp files: https://android.googlesource.com/platform/external/libdrm/+/fa32e29a1fe81e5472aabc65d3aa25a5af5aec55%5E%21/ and wonder if getting that conversion upstream would be a good idea here? Cc: Emil Velikov <emil.l.velikov@gmail.com> Cc: Chad Versace <chad.versace@linux.intel.com> Cc: Marissa Wall <marissaw@google.com> Cc: Sean Paul <seanpaul@google.com> Cc: Rob Herring <rob.herring@linaro.org> Cc: Dan Willemsen <dwillemsen@google.com> Cc: Tomasz Figa <tfiga@google.com> Signed-off-by: John Stultz <john.stultz@linaro.org> --- intel/Android.mk | 2 ++ 1 file changed, 2 insertions(+) -- 2.7.4