On Thursday, 2018-09-20 09:46:48 -0700, Lucas De Marchi wrote: > On Thu, Sep 20, 2018 at 04:58:32PM +0100, Eric Engestrom wrote: > > Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions" > > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > > Cc: Mark Janes <mark.a.janes@xxxxxxxxx> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006 > > Signed-off-by: Eric Engestrom <eric.engestrom@xxxxxxxxx> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > > But for the series, I went to check what went wrong. It seems my script > missed the cases in which we had more than one symbol with the same > name (yes, we do have some static functions that have the same name > of an exported symbol) and cases in which for some reason > cscope didn't return the location of the symbold definition. > > So... I decided to double check if we are now exporting all symbols > that we were previously. My check is with: > > $ git checkout 473e2d2 > $ flags="-D amdgpu=true -D cairo-tests=true -D etnaviv=true -D exynos=true > -D freedreno=true -D freedreno-kgsl=true -D intel=true -D libkms=true > -D man-pages=true -D nouveau=true -D omap=true -D radeon=true > -D tegra=true -D udev=true -D valgrind=true -D vc4=true -D vmwgfx=true" > $ meson _buildold > $ ninja -C _buildold > $ git checkout master > $ ls /tmp/patches/ > 0001-intel-add-missing-drm-public-exports.patch 0003-radeon-add-missing-drm-public-exports.patch > 0002-nouveau-add-missing-drm-public-exports.patch > $ git am /tmp/patches/* > $ meson _buildnew > $ ninja -C _buildnew > $ find _buildnew/ -name '*.so' | while read f; do > fold=${f/_buildnew/_buildold} > nm --dynamic --defined-only $f | grep -i " T " | cut -d' ' -f3 | sort -u > /tmp/new.txt > nm --dynamic --defined-only $fold | grep -i " T " | cut -d' ' -f3 | sort -u > /tmp/old.txt > git diff /tmp/old.txt /tmp/new.txt > done > > So.. we still have the following symbols missing. > > diff --git a/tmp/old.txt b/tmp/new.txt > index 799b63d8..dd0c30c2 100644 > --- a/tmp/old.txt > +++ b/tmp/new.txt > @@ -29,7 +29,6 @@ radeon_cs_need_flush > radeon_cs_print > radeon_cs_set_limit > radeon_cs_space_add_persistent_bo > -radeon_cs_space_check Ha, I missed that one because my grep found the one with the `_with_bo` suffix which was properly exported... :eyeroll: I'll send a v2 in a bit with that added. > radeon_cs_space_check_with_bo > radeon_cs_space_reset_bos > radeon_cs_space_set_flush > diff --git a/tmp/old.txt b/tmp/new.txt > index d846faf0..e08eac77 100644 > --- a/tmp/old.txt > +++ b/tmp/new.txt > @@ -4,13 +4,9 @@ omap_bo_cpu_fini > omap_bo_cpu_prep > omap_bo_del > omap_bo_dmabuf > -omap_bo_from_dmabuf > -omap_bo_from_name > omap_bo_get_name > omap_bo_handle > omap_bo_map > -omap_bo_new > -omap_bo_new_tiled > omap_bo_ref > omap_bo_size > omap_device_del These ones actually do have the drm_public annotation, although it was not written the "normal way", which might be why it's not working? For example: struct omap_bo * drm_public omap_bo_from_name(struct omap_device *dev, uint32_t name) { ... I guess from your script above that they are actually not exported? Could you double-check, and send a patch for these omap ones? Thanks :) > diff --git a/tmp/old.txt b/tmp/new.txt > index fe0dbf39..40a459ad 100644 > --- a/tmp/old.txt > +++ b/tmp/new.txt > @@ -3,7 +3,6 @@ fd_bo_cpu_prep > fd_bo_del > fd_bo_dmabuf > fd_bo_from_dmabuf > -fd_bo_from_fbdev This one is actually all good: drm_public struct fd_bo * fd_bo_from_fbdev(struct fd_pipe *pipe, int fbfd, uint32_t size) It is hidden behind `!HAVE_FREEDRENO_KGSL` which is itself controlled by `-D freedreno-kgsl=false` on meson; make sure you're using the same config for your before/after comparison :) > fd_bo_from_handle > fd_bo_from_name > fd_bo_get_iova > > I hope I'm covering everything now. > > Thanks > Lucas De Marchi > > > --- > > intel/intel_bufmgr_fake.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c > > index 57cbc5365b9f0779dd5a..0cec51f5b836d26e57e0 100644 > > --- a/intel/intel_bufmgr_fake.c > > +++ b/intel/intel_bufmgr_fake.c > > @@ -241,7 +241,7 @@ FENCE_LTE(unsigned a, unsigned b) > > return 0; > > } > > > > -void > > +drm_public void > > drm_intel_bufmgr_fake_set_fence_callback(drm_intel_bufmgr *bufmgr, > > unsigned int (*emit) (void *priv), > > void (*wait) (unsigned int fence, > > @@ -955,7 +955,7 @@ drm_intel_fake_bo_unreference(drm_intel_bo *bo) > > * Set the buffer as not requiring backing store, and instead get the callback > > * invoked whenever it would be set dirty. > > */ > > -void > > +drm_public void > > drm_intel_bo_fake_disable_backing_store(drm_intel_bo *bo, > > void (*invalidate_cb) (drm_intel_bo *bo, > > void *ptr), > > @@ -1409,7 +1409,7 @@ drm_intel_bo_fake_post_submit(drm_intel_bo *bo) > > bo_fake->write_domain = 0; > > } > > > > -void > > +drm_public void > > drm_intel_bufmgr_fake_set_exec_callback(drm_intel_bufmgr *bufmgr, > > int (*exec) (drm_intel_bo *bo, > > unsigned int used, > > -- > > Cheers, > > Eric > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
