* [PATCH libdrm 1/3] intel: add missing drm_public exports
@ 2018-09-20 15:58 Eric Engestrom
2018-09-20 15:58 ` [PATCH libdrm 2/3] nouveau: " Eric Engestrom
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 15:58 UTC (permalink / raw)
To: dri-devel; +Cc: Mark Janes, Lucas De Marchi, Dylan Baker
Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions"
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
---
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH libdrm 2/3] nouveau: add missing drm_public exports
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
@ 2018-09-20 15:58 ` Eric Engestrom
2018-09-20 17:14 ` Dylan Baker
2018-09-20 15:58 ` [PATCH libdrm 3/3] radeon: " Eric Engestrom
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 15:58 UTC (permalink / raw)
To: dri-devel; +Cc: Mark Janes, Lucas De Marchi, Dylan Baker
Fixes: d7320bfcddc596f23fa2 "nouveau: annotate public functions"
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
---
nouveau/nouveau.c | 2 +-
nouveau/pushbuf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c
index 5be0611e0e79451112c6..f18d1426bd4000019e3c 100644
--- a/nouveau/nouveau.c
+++ b/nouveau/nouveau.c
@@ -856,7 +856,7 @@ nouveau_bo_wait(struct nouveau_bo *bo, uint32_t access,
return ret;
}
-int
+drm_public int
nouveau_bo_map(struct nouveau_bo *bo, uint32_t access,
struct nouveau_client *client)
{
diff --git a/nouveau/pushbuf.c b/nouveau/pushbuf.c
index 856ae7ee169b58bffd78..e5f73f0d74c178939c91 100644
--- a/nouveau/pushbuf.c
+++ b/nouveau/pushbuf.c
@@ -728,7 +728,7 @@ nouveau_pushbuf_data(struct nouveau_pushbuf *push, struct nouveau_bo *bo,
}
}
-int
+drm_public int
nouveau_pushbuf_refn(struct nouveau_pushbuf *push,
struct nouveau_pushbuf_refn *refs, int nr)
{
--
Cheers,
Eric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH libdrm 3/3] radeon: add missing drm_public exports
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
2018-09-20 15:58 ` [PATCH libdrm 2/3] nouveau: " Eric Engestrom
@ 2018-09-20 15:58 ` Eric Engestrom
2018-09-20 16:09 ` Michel Dänzer
2018-09-20 16:46 ` [PATCH libdrm 1/3] intel: " Lucas De Marchi
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 15:58 UTC (permalink / raw)
To: dri-devel; +Cc: Mark Janes, Lucas De Marchi, Dylan Baker
Fixes: 9f45264815eff6ebeba3 "radeon: annotate public functions"
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
---
radeon/radeon_bo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/radeon/radeon_bo.c b/radeon/radeon_bo.c
index cd06c26ee152d68f893d..91929532d5bf6e0daca8 100644
--- a/radeon/radeon_bo.c
+++ b/radeon/radeon_bo.c
@@ -67,13 +67,13 @@ drm_public struct radeon_bo *radeon_bo_unref(struct radeon_bo *bo)
return boi->bom->funcs->bo_unref(boi);
}
-int radeon_bo_map(struct radeon_bo *bo, int write)
+drm_public int radeon_bo_map(struct radeon_bo *bo, int write)
{
struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
return boi->bom->funcs->bo_map(boi, write);
}
-int radeon_bo_unmap(struct radeon_bo *bo)
+drm_public int radeon_bo_unmap(struct radeon_bo *bo)
{
struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
return boi->bom->funcs->bo_unmap(boi);
--
Cheers,
Eric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 3/3] radeon: add missing drm_public exports
2018-09-20 15:58 ` [PATCH libdrm 3/3] radeon: " Eric Engestrom
@ 2018-09-20 16:09 ` Michel Dänzer
2018-09-20 17:21 ` Eric Engestrom
0 siblings, 1 reply; 20+ messages in thread
From: Michel Dänzer @ 2018-09-20 16:09 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, Lucas De Marchi, dri-devel, Dylan Baker
On 2018-09-20 5:58 p.m., Eric Engestrom wrote:
> Fixes: 9f45264815eff6ebeba3 "radeon: annotate public functions"
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> ---
> radeon/radeon_bo.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/radeon/radeon_bo.c b/radeon/radeon_bo.c
> index cd06c26ee152d68f893d..91929532d5bf6e0daca8 100644
> --- a/radeon/radeon_bo.c
> +++ b/radeon/radeon_bo.c
> @@ -67,13 +67,13 @@ drm_public struct radeon_bo *radeon_bo_unref(struct radeon_bo *bo)
> return boi->bom->funcs->bo_unref(boi);
> }
>
> -int radeon_bo_map(struct radeon_bo *bo, int write)
> +drm_public int radeon_bo_map(struct radeon_bo *bo, int write)
> {
> struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> return boi->bom->funcs->bo_map(boi, write);
> }
>
> -int radeon_bo_unmap(struct radeon_bo *bo)
> +drm_public int radeon_bo_unmap(struct radeon_bo *bo)
> {
> struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> return boi->bom->funcs->bo_unmap(boi);
>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Tested-by: Michel Dänzer <michel.daenzer@amd.com>
Thanks Eric!
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
2018-09-20 15:58 ` [PATCH libdrm 2/3] nouveau: " Eric Engestrom
2018-09-20 15:58 ` [PATCH libdrm 3/3] radeon: " Eric Engestrom
@ 2018-09-20 16:46 ` Lucas De Marchi
2018-09-20 17:12 ` Eric Engestrom
2018-09-20 17:09 ` Dylan Baker
2018-09-20 17:30 ` Emil Velikov
4 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2018-09-20 16:46 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, dri-devel, Dylan Baker
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@intel.com>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@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
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
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
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
` (2 preceding siblings ...)
2018-09-20 16:46 ` [PATCH libdrm 1/3] intel: " Lucas De Marchi
@ 2018-09-20 17:09 ` Dylan Baker
2018-09-20 17:30 ` Emil Velikov
4 siblings, 0 replies; 20+ messages in thread
From: Dylan Baker @ 2018-09-20 17:09 UTC (permalink / raw)
To: Eric Engestrom, dri-devel; +Cc: Mark Janes, Lucas De Marchi
[-- Attachment #1.1: Type: text/plain, Size: 1949 bytes --]
Quoting Eric Engestrom (2018-09-20 08:58:32)
> Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions"
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> ---
> 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
>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Tested-by: Dylan Baker <dylan@pnwbakers.com>
[-- Attachment #1.2: signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQRxxLdWILx1cItL2yVMlfqrPrBz7AUCW6PUXQAKCRBMlfqrPrBz
7DcyAP9KyGSeGkGxz01dC7EszuYwzVylJNJ/DplzUd4lXDODkQD+O86RSOiih6lb
flYSqSIeeNP5El2aNO79P3Uxk21bEwM=
=95qG
-----END PGP SIGNATURE-----
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 16:46 ` [PATCH libdrm 1/3] intel: " Lucas De Marchi
@ 2018-09-20 17:12 ` Eric Engestrom
2018-09-20 17:29 ` Eric Engestrom
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 17:12 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Mark Janes, dri-devel, Dylan Baker
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@intel.com>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@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
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 2/3] nouveau: add missing drm_public exports
2018-09-20 15:58 ` [PATCH libdrm 2/3] nouveau: " Eric Engestrom
@ 2018-09-20 17:14 ` Dylan Baker
0 siblings, 0 replies; 20+ messages in thread
From: Dylan Baker @ 2018-09-20 17:14 UTC (permalink / raw)
To: Eric Engestrom, dri-devel; +Cc: Mark Janes, Lucas De Marchi
[-- Attachment #1.1: Type: text/plain, Size: 1446 bytes --]
Quoting Eric Engestrom (2018-09-20 08:58:33)
> Fixes: d7320bfcddc596f23fa2 "nouveau: annotate public functions"
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> ---
> nouveau/nouveau.c | 2 +-
> nouveau/pushbuf.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c
> index 5be0611e0e79451112c6..f18d1426bd4000019e3c 100644
> --- a/nouveau/nouveau.c
> +++ b/nouveau/nouveau.c
> @@ -856,7 +856,7 @@ nouveau_bo_wait(struct nouveau_bo *bo, uint32_t access,
> return ret;
> }
>
> -int
> +drm_public int
> nouveau_bo_map(struct nouveau_bo *bo, uint32_t access,
> struct nouveau_client *client)
> {
> diff --git a/nouveau/pushbuf.c b/nouveau/pushbuf.c
> index 856ae7ee169b58bffd78..e5f73f0d74c178939c91 100644
> --- a/nouveau/pushbuf.c
> +++ b/nouveau/pushbuf.c
> @@ -728,7 +728,7 @@ nouveau_pushbuf_data(struct nouveau_pushbuf *push, struct nouveau_bo *bo,
> }
> }
>
> -int
> +drm_public int
> nouveau_pushbuf_refn(struct nouveau_pushbuf *push,
> struct nouveau_pushbuf_refn *refs, int nr)
> {
> --
> Cheers,
> Eric
>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Tested-by: Dylan Baker <dylan@pnwbakers.com>
[-- Attachment #1.2: signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQRxxLdWILx1cItL2yVMlfqrPrBz7AUCW6PVZAAKCRBMlfqrPrBz
7ODWAQCYncUap4j7RkobHgIa9AOMvHZkH9pUTAha585TIsSYXgD+L0AApdMZd1qE
3HfvT/B5WARNm7H9oMdD/oEWuPhxNAU=
=X0xj
-----END PGP SIGNATURE-----
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 3/3] radeon: add missing drm_public exports
2018-09-20 16:09 ` Michel Dänzer
@ 2018-09-20 17:21 ` Eric Engestrom
2018-09-20 18:31 ` Eric Engestrom
0 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 17:21 UTC (permalink / raw)
To: Michel Dänzer; +Cc: Mark Janes, Lucas De Marchi, dri-devel, Dylan Baker
On Thursday, 2018-09-20 18:09:41 +0200, Michel Dänzer wrote:
> On 2018-09-20 5:58 p.m., Eric Engestrom wrote:
> > Fixes: 9f45264815eff6ebeba3 "radeon: annotate public functions"
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> > ---
> > radeon/radeon_bo.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/radeon/radeon_bo.c b/radeon/radeon_bo.c
> > index cd06c26ee152d68f893d..91929532d5bf6e0daca8 100644
> > --- a/radeon/radeon_bo.c
> > +++ b/radeon/radeon_bo.c
> > @@ -67,13 +67,13 @@ drm_public struct radeon_bo *radeon_bo_unref(struct radeon_bo *bo)
> > return boi->bom->funcs->bo_unref(boi);
> > }
> >
> > -int radeon_bo_map(struct radeon_bo *bo, int write)
> > +drm_public int radeon_bo_map(struct radeon_bo *bo, int write)
> > {
> > struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> > return boi->bom->funcs->bo_map(boi, write);
> > }
> >
> > -int radeon_bo_unmap(struct radeon_bo *bo)
> > +drm_public int radeon_bo_unmap(struct radeon_bo *bo)
> > {
> > struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> > return boi->bom->funcs->bo_unmap(boi);
> >
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
> Tested-by: Michel Dänzer <michel.daenzer@amd.com>
Thanks!
radeon_cs_space_check was also missing, but my grep didn't catch it
because radeon_cs_space_check_with_bo matched my weak grep skills...
I added drm_public to it too, can I still apply your tags, or do you
want a v2?
----8<----
diff --git a/radeon/radeon_cs_space.c b/radeon/radeon_cs_space.c
index 08093300827e287f3c0d..039b0414af30c1967fb7 100644
--- a/radeon/radeon_cs_space.c
+++ b/radeon/radeon_cs_space.c
@@ -227,7 +227,7 @@ radeon_cs_space_check_with_bo(struct radeon_cs *cs, struct radeon_bo *bo,
return ret;
}
-int radeon_cs_space_check(struct radeon_cs *cs)
+drm_public int radeon_cs_space_check(struct radeon_cs *cs)
{
struct radeon_cs_int *csi = (struct radeon_cs_int *)cs;
return radeon_cs_check_space_internal(csi, NULL);
---->8----
>
> Thanks Eric!
>
>
> --
> Earthling Michel Dänzer | http://www.amd.com
> Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 17:12 ` Eric Engestrom
@ 2018-09-20 17:29 ` Eric Engestrom
2018-09-20 17:29 ` [PATCH libdrm] omap: fix symbol annotations Eric Engestrom
2018-09-20 17:58 ` [PATCH libdrm 1/3] intel: add missing drm_public exports Lucas De Marchi
2018-09-20 18:12 ` Lucas De Marchi
2 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 17:29 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Mark Janes, dri-devel, Dylan Baker
On Thursday, 2018-09-20 18:12:59 +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@intel.com>
> > > Cc: Mark Janes <mark.a.janes@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> >
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@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 :)
Actually, I went ahead and did that patch anyway, sending it in
a minute.
>
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH libdrm] omap: fix symbol annotations
2018-09-20 17:29 ` Eric Engestrom
@ 2018-09-20 17:29 ` Eric Engestrom
2018-09-20 18:08 ` Lucas De Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 17:29 UTC (permalink / raw)
To: dri-devel; +Cc: Lucas De Marchi
Fixes: f3f7266d94e0354bfd97 "omap: annotate public functions"
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
---
omap/omap_drm.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/omap/omap_drm.c b/omap/omap_drm.c
index 3136e04e4170ac3bb94d..3aed4e0a2f65a9aec6f5 100644
--- a/omap/omap_drm.c
+++ b/omap/omap_drm.c
@@ -128,8 +128,8 @@ drm_public void omap_device_del(struct omap_device *dev)
free(dev);
}
-int
-drm_public omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *value)
+drm_public int
+omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *value)
{
struct drm_omap_param req = {
.param = param,
@@ -146,8 +146,8 @@ drm_public omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *val
return 0;
}
-int
-drm_public omap_set_param(struct omap_device *dev, uint64_t param, uint64_t value)
+drm_public int
+omap_set_param(struct omap_device *dev, uint64_t param, uint64_t value)
{
struct drm_omap_param req = {
.param = param,
@@ -226,8 +226,8 @@ static struct omap_bo * omap_bo_new_impl(struct omap_device *dev,
/* allocate a new (un-tiled) buffer object */
-struct omap_bo *
-drm_public omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
+drm_public struct omap_bo *
+omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
{
union omap_gem_size gsize = {
.bytes = size,
@@ -239,8 +239,8 @@ drm_public omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
}
/* allocate a new buffer object */
-struct omap_bo *
-drm_public omap_bo_new_tiled(struct omap_device *dev, uint32_t width,
+drm_public struct omap_bo *
+omap_bo_new_tiled(struct omap_device *dev, uint32_t width,
uint32_t height, uint32_t flags)
{
union omap_gem_size gsize = {
@@ -281,8 +281,8 @@ static int get_buffer_info(struct omap_bo *bo)
}
/* import a buffer object from DRI2 name */
-struct omap_bo *
-drm_public omap_bo_from_name(struct omap_device *dev, uint32_t name)
+drm_public struct omap_bo *
+omap_bo_from_name(struct omap_device *dev, uint32_t name)
{
struct omap_bo *bo = NULL;
struct drm_gem_open req = {
@@ -315,8 +315,8 @@ drm_public omap_bo_from_name(struct omap_device *dev, uint32_t name)
* fd so caller should close() the fd when it is otherwise done
* with it (even if it is still using the 'struct omap_bo *')
*/
-struct omap_bo *
-drm_public omap_bo_from_dmabuf(struct omap_device *dev, int fd)
+drm_public struct omap_bo *
+omap_bo_from_dmabuf(struct omap_device *dev, int fd)
{
struct omap_bo *bo = NULL;
struct drm_prime_handle req = {
--
Cheers,
Eric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
` (3 preceding siblings ...)
2018-09-20 17:09 ` Dylan Baker
@ 2018-09-20 17:30 ` Emil Velikov
2018-09-20 17:39 ` Eric Engestrom
2018-09-20 17:50 ` Lucas De Marchi
4 siblings, 2 replies; 20+ messages in thread
From: Emil Velikov @ 2018-09-20 17:30 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, Lucas De Marchi, ML dri-devel, Dylan Baker
On 20 September 2018 at 16:58, Eric Engestrom <eric.engestrom@intel.com> wrote:
> Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions"
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> ---
> intel/intel_bufmgr_fake.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
This indicated my earlier concern it's not an issue of drm_public vs
drm_private - people simply forget to run make check.
As said earlier - if you want this in Intel sure, but don't push it onto others.
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 17:30 ` Emil Velikov
@ 2018-09-20 17:39 ` Eric Engestrom
2018-09-20 17:50 ` Lucas De Marchi
1 sibling, 0 replies; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 17:39 UTC (permalink / raw)
To: Emil Velikov; +Cc: Mark Janes, Lucas De Marchi, ML dri-devel, Dylan Baker
On Thursday, 2018-09-20 18:30:54 +0100, Emil Velikov wrote:
> On 20 September 2018 at 16:58, Eric Engestrom <eric.engestrom@intel.com> wrote:
> > Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions"
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> > ---
> > intel/intel_bufmgr_fake.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> This indicated my earlier concern it's not an issue of drm_public vs
> drm_private - people simply forget to run make check.
Actually, this is a limitation of the current symbol check system: it
only checks that nothing is exported when it shouldn't be, but it doesn't
check that what should be exported actually is.
My symbols check series fixes that and would've caught these; I really
need to find the time to finish it :/
> As said earlier - if you want this in Intel sure, but don't push it onto others.
Intel or not isn't the issue here, but I understand your concern.
If someone using one of those broken compilers raises a bug, we might
have to reconsider and revert it, but for now let's just try to fix it :)
>
> Thanks
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 17:30 ` Emil Velikov
2018-09-20 17:39 ` Eric Engestrom
@ 2018-09-20 17:50 ` Lucas De Marchi
1 sibling, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2018-09-20 17:50 UTC (permalink / raw)
To: Emil Velikov; +Cc: Eric Engestrom, Mark Janes, ML dri-devel, Dylan Baker
On Thu, Sep 20, 2018 at 06:30:54PM +0100, Emil Velikov wrote:
> On 20 September 2018 at 16:58, Eric Engestrom <eric.engestrom@intel.com> wrote:
> > Fixes: 36bb0ea47b71d220b31e "intel: annotate public functions"
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> > ---
> > intel/intel_bufmgr_fake.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> This indicated my earlier concern it's not an issue of drm_public vs
> drm_private - people simply forget to run make check.
make check with all build systems was passing, this was not the problem
> As said earlier - if you want this in Intel sure, but don't push it onto others.
I will respond to this in the other thread.
Lucas De Marchi
>
> Thanks
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 17:12 ` Eric Engestrom
2018-09-20 17:29 ` Eric Engestrom
@ 2018-09-20 17:58 ` Lucas De Marchi
2018-09-20 18:12 ` Lucas De Marchi
2 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2018-09-20 17:58 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, dri-devel, Dylan Baker
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@intel.com>
> > > Cc: Mark Janes <mark.a.janes@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> >
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@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 :)
that's why I used a flags variable... to pass the same configuration flags
before and after.
it's drm_public in the !HAVE_FREEDRENO_KGSL case, but is should also be drm_public
when this configure flag is true. In this case the function is defined in
kgsl_bo.c rather than freedreno_bo.c.
Lucas De Marchi
>
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm] omap: fix symbol annotations
2018-09-20 17:29 ` [PATCH libdrm] omap: fix symbol annotations Eric Engestrom
@ 2018-09-20 18:08 ` Lucas De Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2018-09-20 18:08 UTC (permalink / raw)
To: Eric Engestrom; +Cc: dri-devel
On Thu, Sep 20, 2018 at 06:29:13PM +0100, Eric Engestrom wrote:
> Fixes: f3f7266d94e0354bfd97 "omap: annotate public functions"
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
> ---
> omap/omap_drm.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/omap/omap_drm.c b/omap/omap_drm.c
> index 3136e04e4170ac3bb94d..3aed4e0a2f65a9aec6f5 100644
> --- a/omap/omap_drm.c
> +++ b/omap/omap_drm.c
> @@ -128,8 +128,8 @@ drm_public void omap_device_del(struct omap_device *dev)
> free(dev);
> }
>
> -int
> -drm_public omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *value)
> +drm_public int
> +omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *value)
> {
> struct drm_omap_param req = {
> .param = param,
> @@ -146,8 +146,8 @@ drm_public omap_get_param(struct omap_device *dev, uint64_t param, uint64_t *val
> return 0;
> }
>
> -int
> -drm_public omap_set_param(struct omap_device *dev, uint64_t param, uint64_t value)
> +drm_public int
> +omap_set_param(struct omap_device *dev, uint64_t param, uint64_t value)
> {
> struct drm_omap_param req = {
> .param = param,
> @@ -226,8 +226,8 @@ static struct omap_bo * omap_bo_new_impl(struct omap_device *dev,
>
>
> /* allocate a new (un-tiled) buffer object */
> -struct omap_bo *
> -drm_public omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
> +drm_public struct omap_bo *
> +omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
> {
> union omap_gem_size gsize = {
> .bytes = size,
> @@ -239,8 +239,8 @@ drm_public omap_bo_new(struct omap_device *dev, uint32_t size, uint32_t flags)
> }
>
> /* allocate a new buffer object */
> -struct omap_bo *
> -drm_public omap_bo_new_tiled(struct omap_device *dev, uint32_t width,
> +drm_public struct omap_bo *
> +omap_bo_new_tiled(struct omap_device *dev, uint32_t width,
> uint32_t height, uint32_t flags)
> {
> union omap_gem_size gsize = {
> @@ -281,8 +281,8 @@ static int get_buffer_info(struct omap_bo *bo)
> }
>
> /* import a buffer object from DRI2 name */
> -struct omap_bo *
> -drm_public omap_bo_from_name(struct omap_device *dev, uint32_t name)
> +drm_public struct omap_bo *
> +omap_bo_from_name(struct omap_device *dev, uint32_t name)
> {
> struct omap_bo *bo = NULL;
> struct drm_gem_open req = {
> @@ -315,8 +315,8 @@ drm_public omap_bo_from_name(struct omap_device *dev, uint32_t name)
> * fd so caller should close() the fd when it is otherwise done
> * with it (even if it is still using the 'struct omap_bo *')
> */
> -struct omap_bo *
> -drm_public omap_bo_from_dmabuf(struct omap_device *dev, int fd)
> +drm_public struct omap_bo *
> +omap_bo_from_dmabuf(struct omap_device *dev, int fd)
> {
> struct omap_bo *bo = NULL;
> struct drm_prime_handle req = {
> --
> Cheers,
> Eric
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 17:12 ` Eric Engestrom
2018-09-20 17:29 ` Eric Engestrom
2018-09-20 17:58 ` [PATCH libdrm 1/3] intel: add missing drm_public exports Lucas De Marchi
@ 2018-09-20 18:12 ` Lucas De Marchi
2018-09-20 18:30 ` Eric Engestrom
2 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2018-09-20 18:12 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, dri-devel, Dylan Baker
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@intel.com>
> > > Cc: Mark Janes <mark.a.janes@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> >
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@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 :)
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 1/3] intel: add missing drm_public exports
2018-09-20 18:12 ` Lucas De Marchi
@ 2018-09-20 18:30 ` Eric Engestrom
0 siblings, 0 replies; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 18:30 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Mark Janes, dri-devel, Dylan Baker
On Thursday, 2018-09-20 11:12:49 -0700, Lucas De Marchi wrote:
> 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@intel.com>
> > > > Cc: Mark Janes <mark.a.janes@intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > > > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> > >
> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@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 :)
>
> 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.
OK, I pushed everything (including the freedreno one, which was technically
not reviewed, but I'm going home and I'd rather not leave it like that).
>
> thanks for fixing my breakage.
Sure, and thanks for helping me :)
>
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 3/3] radeon: add missing drm_public exports
2018-09-20 17:21 ` Eric Engestrom
@ 2018-09-20 18:31 ` Eric Engestrom
2018-09-21 7:48 ` Michel Dänzer
0 siblings, 1 reply; 20+ messages in thread
From: Eric Engestrom @ 2018-09-20 18:31 UTC (permalink / raw)
To: Michel Dänzer; +Cc: Mark Janes, Lucas De Marchi, dri-devel, Dylan Baker
On Thursday, 2018-09-20 18:21:41 +0100, Eric Engestrom wrote:
> On Thursday, 2018-09-20 18:09:41 +0200, Michel Dänzer wrote:
> > On 2018-09-20 5:58 p.m., Eric Engestrom wrote:
> > > Fixes: 9f45264815eff6ebeba3 "radeon: annotate public functions"
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Cc: Mark Janes <mark.a.janes@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
> > > Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
> > > ---
> > > radeon/radeon_bo.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/radeon/radeon_bo.c b/radeon/radeon_bo.c
> > > index cd06c26ee152d68f893d..91929532d5bf6e0daca8 100644
> > > --- a/radeon/radeon_bo.c
> > > +++ b/radeon/radeon_bo.c
> > > @@ -67,13 +67,13 @@ drm_public struct radeon_bo *radeon_bo_unref(struct radeon_bo *bo)
> > > return boi->bom->funcs->bo_unref(boi);
> > > }
> > >
> > > -int radeon_bo_map(struct radeon_bo *bo, int write)
> > > +drm_public int radeon_bo_map(struct radeon_bo *bo, int write)
> > > {
> > > struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> > > return boi->bom->funcs->bo_map(boi, write);
> > > }
> > >
> > > -int radeon_bo_unmap(struct radeon_bo *bo)
> > > +drm_public int radeon_bo_unmap(struct radeon_bo *bo)
> > > {
> > > struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
> > > return boi->bom->funcs->bo_unmap(boi);
> > >
> >
> > Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
> > Tested-by: Michel Dänzer <michel.daenzer@amd.com>
>
> Thanks!
>
> radeon_cs_space_check was also missing, but my grep didn't catch it
> because radeon_cs_space_check_with_bo matched my weak grep skills...
>
> I added drm_public to it too, can I still apply your tags, or do you
> want a v2?
I ended up pushing it with your r-b and t-b, because I'm going home and
would rather not have left it like that too long :)
Thanks again!
>
> ----8<----
> diff --git a/radeon/radeon_cs_space.c b/radeon/radeon_cs_space.c
> index 08093300827e287f3c0d..039b0414af30c1967fb7 100644
> --- a/radeon/radeon_cs_space.c
> +++ b/radeon/radeon_cs_space.c
> @@ -227,7 +227,7 @@ radeon_cs_space_check_with_bo(struct radeon_cs *cs, struct radeon_bo *bo,
> return ret;
> }
>
> -int radeon_cs_space_check(struct radeon_cs *cs)
> +drm_public int radeon_cs_space_check(struct radeon_cs *cs)
> {
> struct radeon_cs_int *csi = (struct radeon_cs_int *)cs;
> return radeon_cs_check_space_internal(csi, NULL);
> ---->8----
>
> >
> > Thanks Eric!
> >
> >
> > --
> > Earthling Michel Dänzer | http://www.amd.com
> > Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH libdrm 3/3] radeon: add missing drm_public exports
2018-09-20 18:31 ` Eric Engestrom
@ 2018-09-21 7:48 ` Michel Dänzer
0 siblings, 0 replies; 20+ messages in thread
From: Michel Dänzer @ 2018-09-21 7:48 UTC (permalink / raw)
To: Eric Engestrom; +Cc: Mark Janes, Lucas De Marchi, dri-devel, Dylan Baker
On 2018-09-20 8:31 p.m., Eric Engestrom wrote:
> On Thursday, 2018-09-20 18:21:41 +0100, Eric Engestrom wrote:
>> On Thursday, 2018-09-20 18:09:41 +0200, Michel Dänzer wrote:
>>> On 2018-09-20 5:58 p.m., Eric Engestrom wrote:
>>>> Fixes: 9f45264815eff6ebeba3 "radeon: annotate public functions"
>>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>> Cc: Mark Janes <mark.a.janes@intel.com>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108006
>>>> Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
>>>> ---
>>>> radeon/radeon_bo.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/radeon/radeon_bo.c b/radeon/radeon_bo.c
>>>> index cd06c26ee152d68f893d..91929532d5bf6e0daca8 100644
>>>> --- a/radeon/radeon_bo.c
>>>> +++ b/radeon/radeon_bo.c
>>>> @@ -67,13 +67,13 @@ drm_public struct radeon_bo *radeon_bo_unref(struct radeon_bo *bo)
>>>> return boi->bom->funcs->bo_unref(boi);
>>>> }
>>>>
>>>> -int radeon_bo_map(struct radeon_bo *bo, int write)
>>>> +drm_public int radeon_bo_map(struct radeon_bo *bo, int write)
>>>> {
>>>> struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
>>>> return boi->bom->funcs->bo_map(boi, write);
>>>> }
>>>>
>>>> -int radeon_bo_unmap(struct radeon_bo *bo)
>>>> +drm_public int radeon_bo_unmap(struct radeon_bo *bo)
>>>> {
>>>> struct radeon_bo_int *boi = (struct radeon_bo_int *)bo;
>>>> return boi->bom->funcs->bo_unmap(boi);
>>>>
>>>
>>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Tested-by: Michel Dänzer <michel.daenzer@amd.com>
>>
>> Thanks!
>>
>> radeon_cs_space_check was also missing, but my grep didn't catch it
>> because radeon_cs_space_check_with_bo matched my weak grep skills...
>>
>> I added drm_public to it too, can I still apply your tags, or do you
>> want a v2?
>
> I ended up pushing it with your r-b and t-b, because I'm going home and
> would rather not have left it like that too long :)
Good catch and good call, thanks again.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-09-21 7:48 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-20 15:58 [PATCH libdrm 1/3] intel: add missing drm_public exports Eric Engestrom
2018-09-20 15:58 ` [PATCH libdrm 2/3] nouveau: " Eric Engestrom
2018-09-20 17:14 ` Dylan Baker
2018-09-20 15:58 ` [PATCH libdrm 3/3] radeon: " Eric Engestrom
2018-09-20 16:09 ` Michel Dänzer
2018-09-20 17:21 ` Eric Engestrom
2018-09-20 18:31 ` Eric Engestrom
2018-09-21 7:48 ` Michel Dänzer
2018-09-20 16:46 ` [PATCH libdrm 1/3] intel: " Lucas De Marchi
2018-09-20 17:12 ` Eric Engestrom
2018-09-20 17:29 ` Eric Engestrom
2018-09-20 17:29 ` [PATCH libdrm] omap: fix symbol annotations Eric Engestrom
2018-09-20 18:08 ` Lucas De Marchi
2018-09-20 17:58 ` [PATCH libdrm 1/3] intel: add missing drm_public exports Lucas De Marchi
2018-09-20 18:12 ` Lucas De Marchi
2018-09-20 18:30 ` Eric Engestrom
2018-09-20 17:09 ` Dylan Baker
2018-09-20 17:30 ` Emil Velikov
2018-09-20 17:39 ` Eric Engestrom
2018-09-20 17:50 ` Lucas De Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).