On 12/04/2024 14:49, Carsten Haitzler wrote:
On 4/12/24 12:40 PM, Kevin Brodsky wrote:
On 12/04/2024 13:20, Carsten Haitzler wrote:
[...]
[...] @@ -1277,7 +1277,7 @@ struct drm_event {
> struct drm_event_vblank { > struct drm_event base; > - __u64 user_data; > + uintptr_t user_data;
__kernel_uintptr_t surely?
It is surprising that you didn't run into trouble here, because the kernel and userspace views will diverge in purecap (the former will see user_data as 64-bit and the latter as a capability).
hmmm how did i mess this up (and yet vblank tests work)... its the tools/include/uapi/drm/drm.h that should be uintptr_t and this one __kernel_uintptr_t.
We shouldn't use uintptr_t in tools/include either. uintptr_t is 32-bit on a 32-bit arch, but this is always at least 64-bit. __kernel_uintptr_t can be used in those headers as well thanks to [1].
user data for vblank events was originally defined as a ulong - thus uintptr_t is actually correct here. it's called signal in public drm headers - but it is the user_data. vblank is special in drm because of this. :)
But struct drm_event_vblank::user_data is __u64 in include/uapi/drm/drm.h. Are you saying that userspace doesn't use that? In which case, was the uapi header broken all along and should it be fixed upstream?
it's not used. libdrm ships its own drm headers and thus i've patched those separately in libdrm itself. they have this as a ulong. :) it's always been this way... broken by design™ :)
Amazing... but then does it even make sense to bother patching any header in tools/include?
Kevin