On Thu, Sep 20, 2018 at 06:12:59PM +0100, Eric Engestrom wrote: > 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 :) I double checked everything. With your patches applied (including the omap one and the radeon_cs_space_check) and the additional drm_public in fd_bo_from_fbdev, then it passes the symbol check I did with before vs after. thanks for fixing my breakage. Lucas De Marchi > > > 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
