+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?
Lucas De Marchi
the rationale of this change.
Unless you are going to privatize all IS_GEN{n} macros.
(are you?)
2018-09-13 0:03 GMT+08:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
On Wed, Sep 12, 2018 at 08:50:54AM -0700, Rodrigo Vivi wrote:
From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
They're used internally and never meant to be part of the API.
Add the drm_private notation, which should resolve that.
v2: (Rodrigo) Add missing include.
v3: (Rodrigo) Keep includes grouped per Eric suggestion.
Cc: Eric Engestrom <eric.engestrom@xxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Fixes: 4e81d4f9c9b ("intel: add generic functions to check PCI ID")
Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
Acked-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Reviewed-by: Eric Engestrom <eric.engestrom@xxxxxxxxx>
And pushed...
thanks for patch, ack and review ;)
---
intel/intel_chipset.c | 4 ++--
intel/intel_chipset.h | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
index d5c33cc5..5aa4a2f2 100644
--- a/intel/intel_chipset.c
+++ b/intel/intel_chipset.c
@@ -44,7 +44,7 @@ static const struct pci_device {
INTEL_SKL_IDS(9),
};
-bool intel_is_genx(unsigned int devid, int gen)
+drm_private bool intel_is_genx(unsigned int devid, int gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
@@ -66,7 +66,7 @@ bool intel_is_genx(unsigned int devid, int gen)
return false;
}
-bool intel_get_genx(unsigned int devid, int *gen)
+drm_private bool intel_get_genx(unsigned int devid, int *gen)
{
const struct pci_device *p,
*pend = pciids + sizeof(pciids) / sizeof(pciids[0]);
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 9b1e64f1..5db207cc 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -329,9 +329,10 @@
/* New platforms use kernel pci ids */
#include <stdbool.h>
+#include <libdrm_macros.h>
-bool intel_is_genx(unsigned int devid, int gen);
-bool intel_get_genx(unsigned int devid, int *gen);
+drm_private bool intel_is_genx(unsigned int devid, int gen);
+drm_private bool intel_get_genx(unsigned int devid, int *gen);
#define IS_GEN9(devid) intel_is_genx(devid, 9)
#define IS_GEN10(devid) intel_is_genx(devid, 10)
--
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
