[Mesa-dev] [PATCH] Android: fix r300g only build

[Rob Herring]

On Wed, Apr 26, 2017 at 6:39 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
> 2017-04-27 0:37 GMT+02:00 Rob Herring <robh at kernel.org>:
>>
>> On Tue, Apr 25, 2017 at 3:12 PM, Rob Herring <robh at kernel.org> wrote:
>> > On Tue, Apr 25, 2017 at 1:50 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>> >> 2017-04-25 18:21 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
>> >>>
>> >>> On 24 April 2017 at 22:49, Rob Herring <robh at kernel.org> wrote:
>> >>> > On Mon, Apr 24, 2017 at 11:59 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> >>> >> Hi Rob,
>> >>> >>
>> >>> >> On 24 April 2017 at 17:46, Rob Herring <robh at kernel.org> wrote:
>> >>> >>> If r300g is the only radeon driver built, the Android build fails to
>> >>> >>> build:
>> >>> >>>
>> >>> >>> ninja: error:
>> >>> >>> 'out/target/product/linaro_x86_64/obj/STATIC_LIBRARIES/libmesa_pipe_radeon_intermediates/export_includes',
>> >>> >>> needed by
>> >>> >>> 'out/target/product/linaro_x86_64/obj/SHARED_LIBRARIES/gallium_dri_intermediates/import_includes',
>> >>> >>> missing and no known rule to make it
>> >>> >>>
>> >>> >>> This is because the path to build libmesa_pipe_radeon was only getting
>> >>> >>> added for r600g and radeonsi, but the library dependency was added for
>> >>> >>> all radeon drivers. As libmesa_pipe_radeon is not needed for r300g, drop
>> >>> >>> the library dependency.
>> >>> >>>
>> >>> >> I think we want to move libmesa_amdgpu_addrlib in a similar way. The
>> >>> >> lib is used/required by libmesa_winsys_amdgpu which is a r600/radeonsi
>> >>> >> only.
>> >>> >> Can you please build test that and send a patch (or even squash here
>> >>> >> if you prefer)?
>> >>> >
>> >>> > You are right. Looking at this a bit more, I think we want to push all
>> >>> > the "extra" libraries down into the driver makefiles. With that we can
>> >>> > also properly export and import include directories.
>> >>> >
>> >>> >> The patch as-is
>> >>> >> Acked-by: Emil Velikov <emil.velikov at collabora.com>
>> >>> >
>> >>> > This one fixes the build, so can you apply it and I'll send a
>> >>> > follow-up series with further clean-ups. They'll need Mauro's help to
>> >>> > test because I don't have radeonsi building.
>> >>> >
>> >>> Pushed this one, thanks.
>> >>>
>> >>> Fwiw one doesn't need anything r600 or radeonsi related. Just move
>> >>> libmesa_amdgpu_addrlib out of r300 (as we do here for
>> >>> libmesa_pipe_radeon) and see that things link fine.
>> >
>> > The problem is not needing h/w, but radeonsi doesn't build with
>> > mainline AOSP. There's additional LLVM bits needed.
>> >
>> >>> Either way mostly thinking out loud.
>> >>>
>> >>> -Emil
>> >>
>> >>
>> >> Hi Rob, Emil,
>> >>
>> >> I tested building r300g, r600g, radeonsi together and the build is broken,
>> >>
>> >> due to multiple symbols defintion, which happens due to the use of
>> >>
>> >> LOCAL_WHOLE_STATIC_LIBRARIES := \
>> >>     $(gallium_DRIVERS) \
>> >
>> > $(sort $(gallium_DRIVERS)) will fix this as sort removes duplicates. I
>> > hit this in the follow-up I'm working on. Will post it soon.
>> >
>> >> ...
>> >>
>> >> prebuilts/gcc/linux-x86/x86/x86_64-linux-android-4.9/x86_64-linux-android/bin/ld:
>> >> error: out/target/product/x86_64/obj_x86/STATIC_LIBRARIES/libmesa_pipe_radeon_intermediates/libmesa_pipe_radeon.a(cayman_msaa.o):
>> >> multiple definition of 'cayman_emit_msaa_config'
>> >> prebuilts/gcc/linux-x86/x86/x86_64-linux-android-4.9/x86_64-linux-android/bin/ld:
>> >> out/target/product/x86_64/obj_x86/STATIC_LIBRARIES/libmesa_pipe_radeon_intermediates/libmesa_pipe_radeon.a(cayman_msaa.o):
>> >> previous definition here
>> >>
>> >> [repeating for all other symbols]
>> >>
>> >> So now we need revert or correct to support radeonsi also, as before
>> >> it was working,
>> >> and have 'r300g U r600g U radeonsi' support is better that having the
>> >> choice between 'r300g' and 'r300g U r600g'.
>> >>
>> >> BTW, Rob have you tested the llvm 3.9.0 branch I provided for radeonsi
>> >> building purposes, does it build with AOSP/O preview?
>> >
>> > No, sorry I haven't gotten to that.
>>
>> I've now got radeonsi building and the necessary pieces for building
>> llvm in place. master has moved to blueprint files from makefiles, so
>> there wasn't a whole lot of direct reuse from your branch. I've pushed
>> the changes for LLVM[1] and the rest of my clean-up[2]. Any compile
>> testing on L or M in particular would be great.
>>
>> Rob
>>
>> [1] https://github.com/robherring/llvm.git amdgpu
>> [2] https://github.com/robherring/mesa.git android-make-cleanup
>
>
> Thanks Rob, I'll test build in the weekend on M, N
> as in L libdrm and especially drm_gralloc/drm_hwcomposer require a lot of hacks

I would test with current libdrm.

> Following questions about LLVM:
>
> I've seen that you unconditionally added libAMDGPU* whole static
> libraries to libLLVM shared,
> we were just discussing with Chih-Wei about keeping libLLVM gpu
> agnostic as much as possible,
> so if I understood it correctly it would be better to avoid setting
> conditions to add libAMDGPU* based on radeonsi/swrast

Perhaps, but I haven't figured out how to do that with blueprint...

> and you consider safe to add libLLVMExecutionEngine,
> libLLVMRuntimeDyld, libLLVMMMCJIT and libLLVMMOrcJIT to all targets.
>
> We just added for x86/x86_64 and skipped libLLVMOrcJIT for the moment
> is libLLVMOrcJIT necessary with LLVM 3.9.0 to build AOSP/O ?

Maybe not. I haven't gone back to figure out if every hunk is necessary.

> Does the following BP part in shared libLLVM building rules mean that
> those libraries are infact added for all targets besides x86/x86_64?

Yes.

> android: {
> export_include_dirs: ["device/include"],
> + whole_static_libs: [
> + "libLLVMExecutionEngine",
> + "libLLVMRuntimeDyld",
> + "libLLVMMCJIT",
> + "libLLVMOrcJIT",
> + ] + llvm_amdgpu_static_libraries,
> },
>
>
> Thanks
> Mauro