[PATCH libdrm] intel: annotate the intel genx helpers as private

[Chris Wilson]

Quoting Lucas De Marchi (2018-09-13 19:23:49)
> +Chris
> 
> On 9/13/18 12:19 AM, Chih-Wei Huang wrote:
> > Note this patch breaks drm_gralloc:
> > 
> > FAILED: out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/LINKED/libgralloc_drm.so
> > /bin/bash -c "prebuilts/clang/host/linux-x86/clang-4053586/bin/clang++
> > -nostdlib -Wl,-soname,libgralloc_drm.so -Wl,--gc-sections -shared
> > out/target/product/x86_64/obj_x86/lib/crtbegin_so.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm_kms.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm_intel.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm_radeon.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm_nouveau.o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/gralloc_drm_pipe.o
> > -Wl,--whole-archive  -Wl,--no-whole-archive
> > out/target/product/x86_64/obj_x86/STATIC_LIBRARIES/libcompiler_rt-extras_intermediates/libcompiler_rt-extras.a
> >    out/target/product/x86_64/obj_x86/STATIC_LIBRARIES/libatomic_intermediates/libatomic.a
> > out/target/product/x86_64/obj_x86/STATIC_LIBRARIES/libgcc_intermediates/libgcc.a
> > -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -Wl,--build-id=md5
> > -Wl,--warn-shared-textrel -Wl,--fatal-warnings -Wl,--gc-sections
> > -Wl,--hash-style=gnu -Wl,--no-undefined-version -m32  -target
> > i686-linux-android
> > -Bprebuilts/gcc/linux-x86/x86/x86_64-linux-android-4.9/x86_64-linux-android/bin
> > -Wl,--no-undefined out/target/product/x86_64/obj_x86/lib/libdrm.so
> > out/target/product/x86_64/obj_x86/lib/liblog.so
> > out/target/product/x86_64/obj_x86/lib/libcutils.so
> > out/target/product/x86_64/obj_x86/lib/libhardware_legacy.so
> > out/target/product/x86_64/obj_x86/lib/libdrm_intel.so
> > out/target/product/x86_64/obj_x86/lib/libdrm_radeon.so
> > out/target/product/x86_64/obj_x86/lib/libdrm_nouveau.so
> > out/target/product/x86_64/obj_x86/lib/libc++.so
> > out/target/product/x86_64/obj_x86/lib/libc.so
> > out/target/product/x86_64/obj_x86/lib/libm.so
> > out/target/product/x86_64/obj_x86/lib/libdl.so -o
> > out/target/product/x86_64/obj_x86/SHARED_LIBRARIES/libgralloc_drm_intermediates/LINKED/libgralloc_drm.so
> > out/target/product/x86_64/obj_x86/lib/crtend_so.o"
> > external/drm_gralloc/gralloc_drm_intel.c:631: error: undefined
> > reference to 'intel_is_genx'
> > external/drm_gralloc/gralloc_drm_intel.c:630: error: undefined
> > reference to 'intel_get_genx'
> > clang.real: error: linker command failed with exit code 1 (use -v to
> > see invocation)
> > 
> > 
> > That's because drm_gralloc use the IS_GEN9 macro
> > (and other IS_GEN{n} macros) directly.
> > 
> > Since IS_GEN{n} are public APIs, I don't see
> 
> IS_GEN() is *not* public API and should not be. It's an internal macro.
> 
> DESTDIR=/tmp/inst ninja -C build install
> grep -r IS_GEN /tmp/inst/
> Binary file /tmp/inst/usr/local/lib64/libdrm_intel.so.1.0.0 matches
> 
>    [  same thing when using autotools ]
> 
> Grepping https://android.googlesource.com/platform/external/drm_gralloc/ 
> I see IS_GEN* is being used, but I don't see where it's getting it from, 
> unless it's using private headers... Let's see by grepping it:
> 
> $ git grep intel_chipset
> gralloc_drm_intel.c:#include "intel_chipset.h" /* for platform detection 
> macros */
> 
> 
> oh. You're a using a private header :(. How many places and libraries 
> will we need to update to support different platforms? This is crazy.
> AFAICS it only uses that to get the max_stride for each platform... this 
> info should be coming from somewhere else. Chris, any idea here?

Correct. They pulled libdrm into their library, they have the power to do
whatever they like. They should already be statically linking to
libdrm_intel.a and libdrm.a, so redefining drm_private to suit shouldn't
be an issue.

Just looking at the intel code suggests that drm_gralloc is but a toy.
-Chris