[PATCH libdrm 1/3] intel: add missing drm_public exports

[Eric Engestrom]

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 at intel.com>
> > Cc: Mark Janes <mark.a.janes at intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> 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
> >