Re: [PATCH 4.19 098/118] drm/lease: Send a distinct uevent
Greg Kroah-Hartman writes: > 4.19-stable review patch. If anyone has any objections, please let me > know. Acked-by: Keith Packard -- -keith signature.asc Description: PGP signature
Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
Jason Ekstrandwrites: > Yeah, I suppose an application could ask for 10k frames in the future or > something ridiculous like that. For sync_file, people strongly want a > finite time guarantee for security/deadlock reasons. I don't know how > happy they would be with a finite time of 10 minutes... Sure, we've put arbitrary finite limits on vblank counters in other places, so adding some kind of arbitrary limit like a couple of seconds would be entirely reasonable here. The Vulkan API doesn't need any of this complexity at all; the one I'm doing only lets you create a fence for the next vblank. > Ok, that's not expected... Part of the point of sync objects is that > they're re-usable. The client is free to not re-use them or to be very > careful about the recycling process. Heh. I thought the opposite -- lightweight objects that you'd use once and delete. I can certainly change the API to pass in an existing syncobj rather than creating a new one. That would be easier in some ways as it reduces the failure paths a bit. > Is the event the important part or the moderately accurate timestamp? I need the timestamp and sequence number, getting them in an event would mean not having to make another syscall. > We could probably modify drm_syncobj to have a "last signaled" > timestamp that's queryable through an IOCTL. That's exactly what I did, but it only works for these new syncobjs. The fields are global and could be filled in by other bits of the code. > Is the sequence number returned necessary? Will it ever be different from > the one requested? Yes, when the application queues it just slightly too late. The application doesn't actually need either of these values directly, but the system needs them to respond to requests to queue presentation at a specific time, so the vulkan driver wants 'recent' vblank time/sequence data to estimate when a vblank will occur. -- -keith signature.asc Description: PGP signature
Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
Jason Ekstrand writes: > Yeah, I suppose an application could ask for 10k frames in the future or > something ridiculous like that. For sync_file, people strongly want a > finite time guarantee for security/deadlock reasons. I don't know how > happy they would be with a finite time of 10 minutes... Sure, we've put arbitrary finite limits on vblank counters in other places, so adding some kind of arbitrary limit like a couple of seconds would be entirely reasonable here. The Vulkan API doesn't need any of this complexity at all; the one I'm doing only lets you create a fence for the next vblank. > Ok, that's not expected... Part of the point of sync objects is that > they're re-usable. The client is free to not re-use them or to be very > careful about the recycling process. Heh. I thought the opposite -- lightweight objects that you'd use once and delete. I can certainly change the API to pass in an existing syncobj rather than creating a new one. That would be easier in some ways as it reduces the failure paths a bit. > Is the event the important part or the moderately accurate timestamp? I need the timestamp and sequence number, getting them in an event would mean not having to make another syscall. > We could probably modify drm_syncobj to have a "last signaled" > timestamp that's queryable through an IOCTL. That's exactly what I did, but it only works for these new syncobjs. The fields are global and could be filled in by other bits of the code. > Is the sequence number returned necessary? Will it ever be different from > the one requested? Yes, when the application queues it just slightly too late. The application doesn't actually need either of these values directly, but the system needs them to respond to requests to queue presentation at a specific time, so the vulkan driver wants 'recent' vblank time/sequence data to estimate when a vblank will occur. -- -keith signature.asc Description: PGP signature
Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
Jason Ekstrandwrites: > Is the given sequence number guaranteed to be hit in finite time? Certainly, it's a finite value... However, realistically, it's just like all of the other vblank interfaces where you can specify a crazy sequence and block for a very long time. So, no different from the current situation. Of course, the only vulkan API available today only lets you wait for the next vblank, so we'll be asking for a sequence which is one more than the current sequence. > Just to make sure I've got this straight, the client queues a syncobj with > queue_syncobj to fire at a given sequence number. Once the syncobj has > fired (which it can find out by waiting on it), it then calls get_syncobj > to figure out when it was fired? If it cares, it can ask. It might not care at all, in which case it wouldn't have to ask. The syncobj API doesn't provide any direct information about the state of the object in the wait call. > If so, what happens if a syncobj is re-used? Do you just loose the > information? You can't reuse one of these -- the 'queue_syncobj' API creates a new one each time. > If we have a finite time guarantee, I'm kind-of thinking a sync_file might > be better as it's a one-shot and doesn't have the information loss > problem. I'm not actually sure though. It's a one-shot, once signaled, you're done with it. > This whole "wait for a syncobj and then ask when it fired" thing is a bit > odd. I'll have to think on it. Yeah, the event mechanism has the nice feature that you get data with each event. I guess the alternative would be to have a way to get an event when a sync object was ready; perhaps a call which provided a set of syncobjs and delivered a DRM event when any of them was ready? That has a lot of appeal; it turns the poll-like nature of the current API into an epoll-like system. Hrm. -- -keith signature.asc Description: PGP signature
Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
Jason Ekstrand writes: > Is the given sequence number guaranteed to be hit in finite time? Certainly, it's a finite value... However, realistically, it's just like all of the other vblank interfaces where you can specify a crazy sequence and block for a very long time. So, no different from the current situation. Of course, the only vulkan API available today only lets you wait for the next vblank, so we'll be asking for a sequence which is one more than the current sequence. > Just to make sure I've got this straight, the client queues a syncobj with > queue_syncobj to fire at a given sequence number. Once the syncobj has > fired (which it can find out by waiting on it), it then calls get_syncobj > to figure out when it was fired? If it cares, it can ask. It might not care at all, in which case it wouldn't have to ask. The syncobj API doesn't provide any direct information about the state of the object in the wait call. > If so, what happens if a syncobj is re-used? Do you just loose the > information? You can't reuse one of these -- the 'queue_syncobj' API creates a new one each time. > If we have a finite time guarantee, I'm kind-of thinking a sync_file might > be better as it's a one-shot and doesn't have the information loss > problem. I'm not actually sure though. It's a one-shot, once signaled, you're done with it. > This whole "wait for a syncobj and then ask when it fired" thing is a bit > odd. I'll have to think on it. Yeah, the event mechanism has the nice feature that you get data with each event. I guess the alternative would be to have a way to get an event when a sync object was ready; perhaps a call which provided a set of syncobjs and delivered a DRM event when any of them was ready? That has a lot of appeal; it turns the poll-like nature of the current API into an epoll-like system. Hrm. -- -keith signature.asc Description: PGP signature
[PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
(This is an RFC on whether this pair of ioctls seems reasonable. The code compiles, but I haven't tested it as I'm away from home this weekend.) I'm rewriting my implementation of the Vulkan EXT_display_control extension, which provides a way to signal a Vulkan fence at vblank time. I had implemented it using events, but that isn't great as the Vulkan API includes the ability to wait for any of a set of fences to be signaled. As the other Vulkan fences are implemented using dma_fences in the kernel, and (eventually) using syncobj up in user space, it seems reasonable to use syncobjs for everything and hook vblank to those. In any case, I'm proposing two new syncobj/vblank ioctls (the names aren't great; suggestions welcome, as usual): DRM_IOCTL_CRTC_QUEUE_SYNCOBJ Create a new syncobj that will be signaled at (or after) the specified vblank sequence value. This uses the same parameters to specify the target sequence as DRM_IOCTL_CRTC_QUEUE_SEQUENCE. DRM_IOCTL_CRTC_GET_SYNCOBJ Once the above syncobj has been signaled, this ioctl allows the application to find out when that happened, returning both the vblank sequence count and time (in ns). I'd like to hear comments on whether this seems reasonable, or whether I should go in some other direction.
[PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
crtc_queue_syncobj creates a new syncobj that will get signaled at a specified vblank sequence count. crtc_get_syncobj returns the time and vblank sequence count when the syncobj was signaled. The pair of these allows use of syncobjs instead of events for monitoring vblank activity. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_file.c | 5 + drivers/gpu/drm/drm_internal.h | 4 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 13 ++- drivers/gpu/drm/drm_vblank.c | 212 + include/drm/drm_file.h | 11 ++- include/drm/drm_syncobj.h | 13 +++ include/uapi/drm/drm.h | 17 8 files changed, 253 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b3c6e997ccdb..c1ada3fe70b0 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -37,6 +37,7 @@ #include #include +#include #include "drm_legacy.h" #include "drm_internal.h" @@ -711,6 +712,10 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) dma_fence_put(e->fence); } + if (e->syncobj) { + drm_syncobj_put(e->syncobj); + } + if (!e->file_priv) { kfree(e); return; diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index c9d5a6cd4d41..71b9435b5b37 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -75,6 +75,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); +int drm_crtc_queue_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +int drm_crtc_get_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 4aafe4802099..309611fb5d0d 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SYNCOBJ, drm_crtc_queue_syncobj_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SYNCOBJ, drm_crtc_get_syncobj_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index cb4d09c70fd4..e197b007079d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -208,7 +208,7 @@ static const struct dma_fence_ops drm_syncobj_null_fence_ops = { .release = NULL, }; -static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) +int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj, bool signal) { struct drm_syncobj_null_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); @@ -218,7 +218,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) spin_lock_init(>lock); dma_fence_init(>base, _syncobj_null_fence_ops, >lock, 0, 0); - dma_fence_signal(>base); + if (signal) + dma_fence_signal(>base); drm_syncobj_replace_fence(syncobj, >base); @@ -226,6 +227,7 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) return 0; } +EXPORT_SYMBOL(drm_syncobj_assign_null_handle); int drm_syncobj_find_fence(struct drm_file *file_private, u32 handle, @@ -283,7 +285,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, spin_lock_init(>lock); if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) { - ret = drm_syncobj_assign_null_handle(syncobj); + ret = drm_syncobj_assign_null_handle(syncobj, true); if (ret < 0) { drm_syncobj_put(syncobj); return ret; @@ -341,7 +343,7 @@ static int drm_syncobj_create_as_handle(struct drm_file *file_private, return ret; } -static int drm_syncobj_destroy(struc
[PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
(This is an RFC on whether this pair of ioctls seems reasonable. The code compiles, but I haven't tested it as I'm away from home this weekend.) I'm rewriting my implementation of the Vulkan EXT_display_control extension, which provides a way to signal a Vulkan fence at vblank time. I had implemented it using events, but that isn't great as the Vulkan API includes the ability to wait for any of a set of fences to be signaled. As the other Vulkan fences are implemented using dma_fences in the kernel, and (eventually) using syncobj up in user space, it seems reasonable to use syncobjs for everything and hook vblank to those. In any case, I'm proposing two new syncobj/vblank ioctls (the names aren't great; suggestions welcome, as usual): DRM_IOCTL_CRTC_QUEUE_SYNCOBJ Create a new syncobj that will be signaled at (or after) the specified vblank sequence value. This uses the same parameters to specify the target sequence as DRM_IOCTL_CRTC_QUEUE_SEQUENCE. DRM_IOCTL_CRTC_GET_SYNCOBJ Once the above syncobj has been signaled, this ioctl allows the application to find out when that happened, returning both the vblank sequence count and time (in ns). I'd like to hear comments on whether this seems reasonable, or whether I should go in some other direction.
[PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
crtc_queue_syncobj creates a new syncobj that will get signaled at a specified vblank sequence count. crtc_get_syncobj returns the time and vblank sequence count when the syncobj was signaled. The pair of these allows use of syncobjs instead of events for monitoring vblank activity. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_file.c | 5 + drivers/gpu/drm/drm_internal.h | 4 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 13 ++- drivers/gpu/drm/drm_vblank.c | 212 + include/drm/drm_file.h | 11 ++- include/drm/drm_syncobj.h | 13 +++ include/uapi/drm/drm.h | 17 8 files changed, 253 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b3c6e997ccdb..c1ada3fe70b0 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -37,6 +37,7 @@ #include #include +#include #include "drm_legacy.h" #include "drm_internal.h" @@ -711,6 +712,10 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) dma_fence_put(e->fence); } + if (e->syncobj) { + drm_syncobj_put(e->syncobj); + } + if (!e->file_priv) { kfree(e); return; diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index c9d5a6cd4d41..71b9435b5b37 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -75,6 +75,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); +int drm_crtc_queue_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +int drm_crtc_get_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 4aafe4802099..309611fb5d0d 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SYNCOBJ, drm_crtc_queue_syncobj_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SYNCOBJ, drm_crtc_get_syncobj_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index cb4d09c70fd4..e197b007079d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -208,7 +208,7 @@ static const struct dma_fence_ops drm_syncobj_null_fence_ops = { .release = NULL, }; -static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) +int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj, bool signal) { struct drm_syncobj_null_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); @@ -218,7 +218,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) spin_lock_init(>lock); dma_fence_init(>base, _syncobj_null_fence_ops, >lock, 0, 0); - dma_fence_signal(>base); + if (signal) + dma_fence_signal(>base); drm_syncobj_replace_fence(syncobj, >base); @@ -226,6 +227,7 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) return 0; } +EXPORT_SYMBOL(drm_syncobj_assign_null_handle); int drm_syncobj_find_fence(struct drm_file *file_private, u32 handle, @@ -283,7 +285,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, spin_lock_init(>lock); if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) { - ret = drm_syncobj_assign_null_handle(syncobj); + ret = drm_syncobj_assign_null_handle(syncobj, true); if (ret < 0) { drm_syncobj_put(syncobj); return ret; @@ -341,7 +343,7 @@ static int drm_syncobj_create_as_handle(struct drm_file *file_private, return ret; } -static int drm_syncobj_destroy(struct drm_file *file_pri
Re: [PATCH] USB: chaoskey: Use kasprintf() over strcpy()/strcat()
Kees Cook <keesc...@chromium.org> writes: > Instead of kmalloc() with manually calculated values followed by > multiple strcpy()/strcat() calls, just fold it all into a single > kasprintf() call. > > Signed-off-by: Kees Cook <keesc...@chromium.org> Reviewed-by: Keith Packard <kei...@keithp.com> -- -keith signature.asc Description: PGP signature
Re: [PATCH] USB: chaoskey: Use kasprintf() over strcpy()/strcat()
Kees Cook writes: > Instead of kmalloc() with manually calculated values followed by > multiple strcpy()/strcat() calls, just fold it all into a single > kasprintf() call. > > Signed-off-by: Kees Cook Reviewed-by: Keith Packard -- -keith signature.asc Description: PGP signature
Re: [PATCH] hwrng: Explicitly drop current_rng when unregistering rng device
Tom Lendackywrites: > A similar fix was already in the cryptodev tree and I thought was part of > a recent pull request. Yup, just saw it on master. I've checked and it will do pretty much the same thing as my patch, with the addition of clearing cur_rng_set_by_user, which is necessary. Thanks for looking at my patch! -- -keith signature.asc Description: PGP signature
Re: [PATCH] hwrng: Explicitly drop current_rng when unregistering rng device
Tom Lendacky writes: > A similar fix was already in the cryptodev tree and I thought was part of > a recent pull request. Yup, just saw it on master. I've checked and it will do pretty much the same thing as my patch, with the addition of clearing cur_rng_set_by_user, which is necessary. Thanks for looking at my patch! -- -keith signature.asc Description: PGP signature
[PATCH] hwrng: Explicitly drop current_rng when unregistering rng device
When the currently in-use hw rng is being removed from the system, explicitly drop it before using enable_best_rng as enable_best_rng will not do anything if the list of avaialble RNGs is empty. Without this, the last RNG removed from the system will never get cleaned up and hwrng_unregister will hang. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/char/hw_random/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 657b8770b6b9..8df3f9f147e2 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -515,8 +515,10 @@ void hwrng_unregister(struct hwrng *rng) mutex_lock(_mutex); list_del(>list); - if (current_rng == rng) + if (current_rng == rng) { + drop_current_rng(); enable_best_rng(); + } if (list_empty(_list)) { mutex_unlock(_mutex); -- 2.15.1
[PATCH] hwrng: Explicitly drop current_rng when unregistering rng device
When the currently in-use hw rng is being removed from the system, explicitly drop it before using enable_best_rng as enable_best_rng will not do anything if the list of avaialble RNGs is empty. Without this, the last RNG removed from the system will never get cleaned up and hwrng_unregister will hang. Signed-off-by: Keith Packard --- drivers/char/hw_random/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 657b8770b6b9..8df3f9f147e2 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -515,8 +515,10 @@ void hwrng_unregister(struct hwrng *rng) mutex_lock(_mutex); list_del(>list); - if (current_rng == rng) + if (current_rng == rng) { + drop_current_rng(); enable_best_rng(); + } if (list_empty(_list)) { mutex_unlock(_mutex); -- 2.15.1
Re: [PATCH] drm: Check for lessee in DROP_MASTER ioctl
Daniel Vetter <dan...@ffwll.ch> writes: > On Thu, Jan 18, 2018 at 05:51:59PM -0800, Keith Packard wrote: >> Don't let a lessee control what the current DRM master is set to; >> that's the job of the "real" master. Otherwise, the lessee would >> disable all access to master operations for the owner and all lessees >> under it. >> >> This matches the same check made in the SET_MASTER ioctl. >> >> Signed-off-by: Keith Packard <kei...@keithp.com> > > Similar check for setmaster already exists, so looks all good. Do we have > an igt for all this? Iirc there was one floating around, but no idea > what's the status. Might also be good to resubmit them so i915 CI can run > the tests (now that the code has landed). I've got IGT tests for leasing which have been posted to dri-devel but I don't think they've been reviewed. Looks like they could use some more test cases; I didn't catch this one until I was playing with my 'xlease' hack, which runs the X server on a leased FD. > On the patch itself, minus lack of testcases: > > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> Thanks! -- -keith signature.asc Description: PGP signature
Re: [PATCH] drm: Check for lessee in DROP_MASTER ioctl
Daniel Vetter writes: > On Thu, Jan 18, 2018 at 05:51:59PM -0800, Keith Packard wrote: >> Don't let a lessee control what the current DRM master is set to; >> that's the job of the "real" master. Otherwise, the lessee would >> disable all access to master operations for the owner and all lessees >> under it. >> >> This matches the same check made in the SET_MASTER ioctl. >> >> Signed-off-by: Keith Packard > > Similar check for setmaster already exists, so looks all good. Do we have > an igt for all this? Iirc there was one floating around, but no idea > what's the status. Might also be good to resubmit them so i915 CI can run > the tests (now that the code has landed). I've got IGT tests for leasing which have been posted to dri-devel but I don't think they've been reviewed. Looks like they could use some more test cases; I didn't catch this one until I was playing with my 'xlease' hack, which runs the X server on a leased FD. > On the patch itself, minus lack of testcases: > > Reviewed-by: Daniel Vetter Thanks! -- -keith signature.asc Description: PGP signature
Re: [PATCH] USB: misc: chaoskey: Use true and false for boolean values
"Gustavo A. R. Silva" <gust...@embeddedor.com> writes: > Assign true or false to boolean variables instead of an integer value. > > This issue was detected with the help of Coccinelle. > > Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> Reviewed-by: Keith Packard <kei...@keithp.com> -- -keith signature.asc Description: PGP signature
Re: [PATCH] USB: misc: chaoskey: Use true and false for boolean values
"Gustavo A. R. Silva" writes: > Assign true or false to boolean variables instead of an integer value. > > This issue was detected with the help of Coccinelle. > > Signed-off-by: Gustavo A. R. Silva Reviewed-by: Keith Packard -- -keith signature.asc Description: PGP signature
[PATCH] drm: Check for lessee in DROP_MASTER ioctl
Don't let a lessee control what the current DRM master is set to; that's the job of the "real" master. Otherwise, the lessee would disable all access to master operations for the owner and all lessees under it. This matches the same check made in the SET_MASTER ioctl. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_auth.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index aad468d170a7..d9c0f7573905 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -230,6 +230,12 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, if (!dev->master) goto out_unlock; + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = 0; drm_drop_master(dev, file_priv); out_unlock: -- 2.15.1
[PATCH] drm: Check for lessee in DROP_MASTER ioctl
Don't let a lessee control what the current DRM master is set to; that's the job of the "real" master. Otherwise, the lessee would disable all access to master operations for the owner and all lessees under it. This matches the same check made in the SET_MASTER ioctl. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_auth.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index aad468d170a7..d9c0f7573905 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -230,6 +230,12 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, if (!dev->master) goto out_unlock; + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = 0; drm_drop_master(dev, file_priv); out_unlock: -- 2.15.1
[PATCH] drm: move lease init after validation in drm_lease_create
Patch bd36d3bab2e3d08f80766c86487090dbceed4651 fixed a deadlock in the failure path of drm_lease_create. This made the partially initialized lease object visible for a short window of time. To avoid having the lessee state appear transiently, I've rearranged the code so that the lessor fields are not filled in until the parameters are all validated and the function will succeed. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_lease.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 59849f02e2ad..1402c0e71b03 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -220,17 +220,6 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr mutex_lock(>mode_config.idr_mutex); - /* Insert the new lessee into the tree */ - id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); - if (id < 0) { - error = id; - goto out_lessee; - } - - lessee->lessee_id = id; - lessee->lessor = drm_master_get(lessor); - list_add_tail(>lessee_list, >lessees); - idr_for_each_entry(leases, entry, object) { error = 0; if (!idr_find(>mode_config.crtc_idr, object)) @@ -246,6 +235,17 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr } } + /* Insert the new lessee into the tree */ + id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); + if (id < 0) { + error = id; + goto out_lessee; + } + + lessee->lessee_id = id; + lessee->lessor = drm_master_get(lessor); + list_add_tail(>lessee_list, >lessees); + /* Move the leases over */ lessee->leases = *leases; DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id, lessee, lessor->lessee_id, lessor); -- 2.15.1
[PATCH] drm: move lease init after validation in drm_lease_create
Patch bd36d3bab2e3d08f80766c86487090dbceed4651 fixed a deadlock in the failure path of drm_lease_create. This made the partially initialized lease object visible for a short window of time. To avoid having the lessee state appear transiently, I've rearranged the code so that the lessor fields are not filled in until the parameters are all validated and the function will succeed. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_lease.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 59849f02e2ad..1402c0e71b03 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -220,17 +220,6 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr mutex_lock(>mode_config.idr_mutex); - /* Insert the new lessee into the tree */ - id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); - if (id < 0) { - error = id; - goto out_lessee; - } - - lessee->lessee_id = id; - lessee->lessor = drm_master_get(lessor); - list_add_tail(>lessee_list, >lessees); - idr_for_each_entry(leases, entry, object) { error = 0; if (!idr_find(>mode_config.crtc_idr, object)) @@ -246,6 +235,17 @@ static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr } } + /* Insert the new lessee into the tree */ + id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL); + if (id < 0) { + error = id; + goto out_lessee; + } + + lessee->lessee_id = id; + lessee->lessor = drm_master_get(lessor); + list_add_tail(>lessee_list, >lessees); + /* Move the leases over */ lessee->leases = *leases; DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id, lessee, lessor->lessee_id, lessor); -- 2.15.1
[PATCH] drm: Update edid-derived drm_display_info fields at edid property set [v2]
There are a set of values in the drm_display_info structure for each connector which hold information derived from EDID. These are computed in drm_add_display_info. Before this patch, that was only called in drm_add_edid_modes. This meant that they were only set when EDID was present and never reset when EDID was not, as happened when the display was disconnected. One of these fields, non_desktop, is used from drm_mode_connector_update_edid_property, the function responsible for assigning the new edid value to the application-visible property. Various drivers call these two functions (drm_add_edid_modes and drm_mode_connector_update_edid_property) in different orders. This means that even when EDID is present, the drm_display_info fields may not have been computed at the time that drm_mode_connector_update_edid_property used the non_desktop value to set the non_desktop property. I've added a public function (drm_reset_display_info) that resets the drm_display_info field values to default values and then made the drm_add_display_info function public. These two functions are now called directly from drm_mode_connector_update_edid_property so that the drm_display_info fields are always computed from the current EDID information before being used in that function. This means that the drm_display_info values are often computed twice, once when the EDID property it set and a second time when EDID is used to compute modes for the device. The alternative would be to uniformly ensure that the values were computed once before being used, which would require that all drivers reliably invoke the two paths in the same order. The computation is inexpensive enough that it seems more maintainable in the long term to simply compute them in both paths. The API to drm_add_display_info has been changed so that it no longer takes the set of edid-based quirks as a parameter. Rather, it now computes those quirks itself and returns them for further use by drm_add_edid_modes. This patch also includes a number of 'const' additions caused by drm_mode_connector_update_edid_property taking a 'const struct edid *' parameter and wanting to pass that along to drm_add_display_info. v2: after review by Daniel Vetter <daniel.vet...@ffwll.ch> Removed EXPORT_SYMBOL_GPL for drm_reset_display_info and drm_add_display_info. Added FIXME in drm_mode_connector_update_edid_property about potentially merging that with drm_add_edid_modes to avoid the need for two driver calls. Signed-off-by: Keith Packard <kei...@keithp.com> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> --- drivers/gpu/drm/drm_connector.c | 13 ++ drivers/gpu/drm/drm_edid.c | 53 ++--- include/drm/drm_edid.h | 2 ++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 25f4b2e9a44f..83c01f359d55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1207,6 +1207,19 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + /* Set the display info, using edid if available, otherwise +* reseting the values to defaults. This duplicates the work +* done in drm_add_edid_modes, but that function is not +* consistently called before this one in all drivers and the +* computation is cheap enough that it seems better to +* duplicate it rather than attempt to ensure some arbitrary +* ordering of calls. +*/ + if (edid) + drm_add_display_info(connector, edid); + else + drm_reset_display_info(connector); + drm_object_property_set_value(>base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 524eace3d460..365901c1c33c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1731,7 +1731,7 @@ EXPORT_SYMBOL(drm_edid_duplicate); * * Returns true if @vendor is in @edid, false otherwise */ -static bool edid_vendor(struct edid *edid, const char *vendor) +static bool edid_vendor(const struct edid *edid, const char *vendor) { char edid_vendor[3]; @@ -1749,7 +1749,7 @@ static bool edid_vendor(struct edid *edid, const char *vendor) * * This tells subsequent routines what fixes they need to apply. */ -static u32 edid_get_quirks(struct edid *edid) +static u32 edid_get_quirks(const struct edid *edid) { const struct edid_quirk *quirk; int i; @@ -2813,7 +2813,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_ed
[PATCH] drm: Update edid-derived drm_display_info fields at edid property set [v2]
There are a set of values in the drm_display_info structure for each connector which hold information derived from EDID. These are computed in drm_add_display_info. Before this patch, that was only called in drm_add_edid_modes. This meant that they were only set when EDID was present and never reset when EDID was not, as happened when the display was disconnected. One of these fields, non_desktop, is used from drm_mode_connector_update_edid_property, the function responsible for assigning the new edid value to the application-visible property. Various drivers call these two functions (drm_add_edid_modes and drm_mode_connector_update_edid_property) in different orders. This means that even when EDID is present, the drm_display_info fields may not have been computed at the time that drm_mode_connector_update_edid_property used the non_desktop value to set the non_desktop property. I've added a public function (drm_reset_display_info) that resets the drm_display_info field values to default values and then made the drm_add_display_info function public. These two functions are now called directly from drm_mode_connector_update_edid_property so that the drm_display_info fields are always computed from the current EDID information before being used in that function. This means that the drm_display_info values are often computed twice, once when the EDID property it set and a second time when EDID is used to compute modes for the device. The alternative would be to uniformly ensure that the values were computed once before being used, which would require that all drivers reliably invoke the two paths in the same order. The computation is inexpensive enough that it seems more maintainable in the long term to simply compute them in both paths. The API to drm_add_display_info has been changed so that it no longer takes the set of edid-based quirks as a parameter. Rather, it now computes those quirks itself and returns them for further use by drm_add_edid_modes. This patch also includes a number of 'const' additions caused by drm_mode_connector_update_edid_property taking a 'const struct edid *' parameter and wanting to pass that along to drm_add_display_info. v2: after review by Daniel Vetter Removed EXPORT_SYMBOL_GPL for drm_reset_display_info and drm_add_display_info. Added FIXME in drm_mode_connector_update_edid_property about potentially merging that with drm_add_edid_modes to avoid the need for two driver calls. Signed-off-by: Keith Packard Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_connector.c | 13 ++ drivers/gpu/drm/drm_edid.c | 53 ++--- include/drm/drm_edid.h | 2 ++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 25f4b2e9a44f..83c01f359d55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1207,6 +1207,19 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + /* Set the display info, using edid if available, otherwise +* reseting the values to defaults. This duplicates the work +* done in drm_add_edid_modes, but that function is not +* consistently called before this one in all drivers and the +* computation is cheap enough that it seems better to +* duplicate it rather than attempt to ensure some arbitrary +* ordering of calls. +*/ + if (edid) + drm_add_display_info(connector, edid); + else + drm_reset_display_info(connector); + drm_object_property_set_value(>base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 524eace3d460..365901c1c33c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1731,7 +1731,7 @@ EXPORT_SYMBOL(drm_edid_duplicate); * * Returns true if @vendor is in @edid, false otherwise */ -static bool edid_vendor(struct edid *edid, const char *vendor) +static bool edid_vendor(const struct edid *edid, const char *vendor) { char edid_vendor[3]; @@ -1749,7 +1749,7 @@ static bool edid_vendor(struct edid *edid, const char *vendor) * * This tells subsequent routines what fixes they need to apply. */ -static u32 edid_get_quirks(struct edid *edid) +static u32 edid_get_quirks(const struct edid *edid) { const struct edid_quirk *quirk; int i; @@ -2813,7 +2813,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_edid_extension(struct edid *edid, int ext_id) +static u8 *drm_find_edid_extension(const
[PATCH] drm: Update edid-derived drm_display_info fields at edid property set
There are a set of values in the drm_display_info structure for each connector which hold information derived from EDID. These are computed in drm_add_display_info. Before this patch, that was only called in drm_add_edid_modes. This meant that they were only set when EDID was present and never reset when EDID was not, as happened when the display was disconnected. One of these fields, non_desktop, is used from drm_mode_connector_update_edid_property, the function responsible for assigning the new edid value to the application-visible property. Various drivers call these two functions (drm_add_edid_modes and drm_mode_connector_update_edid_property) in different orders. This means that even when EDID is present, the drm_display_info fields may not have been computed at the time that drm_mode_connector_update_edid_property used the non_desktop value to set the non_desktop property. I've added a public function (drm_reset_display_info) that resets the drm_display_info field values to default values and then made the drm_add_display_info function public. These two functions are now called directly from drm_mode_connector_update_edid_property so that the drm_display_info fields are always computed from the current EDID information before being used in that function. This means that the drm_display_info values are often computed twice, once when the EDID property it set and a second time when EDID is used to compute modes for the device. The alternative would be to uniformly ensure that the values were computed once before being used, which would require that all drivers reliably invoke the two paths in the same order. The computation is inexpensive enough that it seems more maintainable in the long term to simply compute them in both paths. The API to drm_add_display_info has been changed so that it no longer takes the set of edid-based quirks as a parameter. Rather, it now computes those quirks itself and returns them for further use by drm_add_edid_modes. This patch also includes a number of 'const' additions caused by drm_mode_connector_update_edid_property taking a 'const struct edid *' parameter and wanting to pass that along to drm_add_display_info. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_connector.c | 13 ++ drivers/gpu/drm/drm_edid.c | 53 ++--- include/drm/drm_edid.h | 2 ++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 25f4b2e9a44f..83c01f359d55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1207,6 +1207,19 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + /* Set the display info, using edid if available, otherwise +* reseting the values to defaults. This duplicates the work +* done in drm_add_edid_modes, but that function is not +* consistently called before this one in all drivers and the +* computation is cheap enough that it seems better to +* duplicate it rather than attempt to ensure some arbitrary +* ordering of calls. +*/ + if (edid) + drm_add_display_info(connector, edid); + else + drm_reset_display_info(connector); + drm_object_property_set_value(>base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 524eace3d460..365901c1c33c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1731,7 +1731,7 @@ EXPORT_SYMBOL(drm_edid_duplicate); * * Returns true if @vendor is in @edid, false otherwise */ -static bool edid_vendor(struct edid *edid, const char *vendor) +static bool edid_vendor(const struct edid *edid, const char *vendor) { char edid_vendor[3]; @@ -1749,7 +1749,7 @@ static bool edid_vendor(struct edid *edid, const char *vendor) * * This tells subsequent routines what fixes they need to apply. */ -static u32 edid_get_quirks(struct edid *edid) +static u32 edid_get_quirks(const struct edid *edid) { const struct edid_quirk *quirk; int i; @@ -2813,7 +2813,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_edid_extension(struct edid *edid, int ext_id) +static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) { u8 *edid_ext = NULL; int i; @@ -2835,12 +2835,12 @@ static u8 *drm_find_edid_extension(struct edid *edid, int ext_id) return edid_ext; } -static u8 *drm_find_cea_extension(struct edid *edid) +static u8 *drm_find_cea_extension(const struct edid *edid)
[PATCH] drm: Update edid-derived drm_display_info fields at edid property set
There are a set of values in the drm_display_info structure for each connector which hold information derived from EDID. These are computed in drm_add_display_info. Before this patch, that was only called in drm_add_edid_modes. This meant that they were only set when EDID was present and never reset when EDID was not, as happened when the display was disconnected. One of these fields, non_desktop, is used from drm_mode_connector_update_edid_property, the function responsible for assigning the new edid value to the application-visible property. Various drivers call these two functions (drm_add_edid_modes and drm_mode_connector_update_edid_property) in different orders. This means that even when EDID is present, the drm_display_info fields may not have been computed at the time that drm_mode_connector_update_edid_property used the non_desktop value to set the non_desktop property. I've added a public function (drm_reset_display_info) that resets the drm_display_info field values to default values and then made the drm_add_display_info function public. These two functions are now called directly from drm_mode_connector_update_edid_property so that the drm_display_info fields are always computed from the current EDID information before being used in that function. This means that the drm_display_info values are often computed twice, once when the EDID property it set and a second time when EDID is used to compute modes for the device. The alternative would be to uniformly ensure that the values were computed once before being used, which would require that all drivers reliably invoke the two paths in the same order. The computation is inexpensive enough that it seems more maintainable in the long term to simply compute them in both paths. The API to drm_add_display_info has been changed so that it no longer takes the set of edid-based quirks as a parameter. Rather, it now computes those quirks itself and returns them for further use by drm_add_edid_modes. This patch also includes a number of 'const' additions caused by drm_mode_connector_update_edid_property taking a 'const struct edid *' parameter and wanting to pass that along to drm_add_display_info. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_connector.c | 13 ++ drivers/gpu/drm/drm_edid.c | 53 ++--- include/drm/drm_edid.h | 2 ++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 25f4b2e9a44f..83c01f359d55 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1207,6 +1207,19 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + /* Set the display info, using edid if available, otherwise +* reseting the values to defaults. This duplicates the work +* done in drm_add_edid_modes, but that function is not +* consistently called before this one in all drivers and the +* computation is cheap enough that it seems better to +* duplicate it rather than attempt to ensure some arbitrary +* ordering of calls. +*/ + if (edid) + drm_add_display_info(connector, edid); + else + drm_reset_display_info(connector); + drm_object_property_set_value(>base, dev->mode_config.non_desktop_property, connector->display_info.non_desktop); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 524eace3d460..365901c1c33c 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1731,7 +1731,7 @@ EXPORT_SYMBOL(drm_edid_duplicate); * * Returns true if @vendor is in @edid, false otherwise */ -static bool edid_vendor(struct edid *edid, const char *vendor) +static bool edid_vendor(const struct edid *edid, const char *vendor) { char edid_vendor[3]; @@ -1749,7 +1749,7 @@ static bool edid_vendor(struct edid *edid, const char *vendor) * * This tells subsequent routines what fixes they need to apply. */ -static u32 edid_get_quirks(struct edid *edid) +static u32 edid_get_quirks(const struct edid *edid) { const struct edid_quirk *quirk; int i; @@ -2813,7 +2813,7 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_edid_extension(struct edid *edid, int ext_id) +static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) { u8 *edid_ext = NULL; int i; @@ -2835,12 +2835,12 @@ static u8 *drm_find_edid_extension(struct edid *edid, int ext_id) return edid_ext; } -static u8 *drm_find_cea_extension(struct edid *edid) +static u8 *drm_find_cea_extension(const struct edid *edid) {
[PATCH 2/3] drm/fb: add support for not enabling fbcon on non-desktop displays [v2]
From: Dave AirlieWe don't want fbcon to get used on non-desktop dislays, don't pass them as enabled connectors to the fb helper setup. This prevents my HMD from getting disorted fbcon, and from affecting other displays console. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_fb_helper.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 116d1f1337c7..07374008f146 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2033,6 +2033,9 @@ static bool drm_connector_enabled(struct drm_connector *connector, bool strict) { bool enable; + if (connector->display_info.non_desktop) + return false; + if (strict) enable = connector->status == connector_status_connected; else @@ -2052,7 +2055,8 @@ static void drm_enable_connectors(struct drm_fb_helper *fb_helper, connector = fb_helper->connector_info[i]->connector; enabled[i] = drm_connector_enabled(connector, true); DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id, - enabled[i] ? "yes" : "no"); + connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no"); + any_enabled |= enabled[i]; } -- 2.15.0.rc0
[PATCH 0/3] drm: Add connector info/property for non-desktop [v2]
This is the same as the series for non-standard displays but uses the phrase 'non-desktop' instead. No functional changes.
[PATCH 2/3] drm/fb: add support for not enabling fbcon on non-desktop displays [v2]
From: Dave Airlie We don't want fbcon to get used on non-desktop dislays, don't pass them as enabled connectors to the fb helper setup. This prevents my HMD from getting disorted fbcon, and from affecting other displays console. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_fb_helper.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 116d1f1337c7..07374008f146 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2033,6 +2033,9 @@ static bool drm_connector_enabled(struct drm_connector *connector, bool strict) { bool enable; + if (connector->display_info.non_desktop) + return false; + if (strict) enable = connector->status == connector_status_connected; else @@ -2052,7 +2055,8 @@ static void drm_enable_connectors(struct drm_fb_helper *fb_helper, connector = fb_helper->connector_info[i]->connector; enabled[i] = drm_connector_enabled(connector, true); DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id, - enabled[i] ? "yes" : "no"); + connector->display_info.non_desktop ? "non desktop" : enabled[i] ? "yes" : "no"); + any_enabled |= enabled[i]; } -- 2.15.0.rc0
[PATCH 0/3] drm: Add connector info/property for non-desktop [v2]
This is the same as the series for non-standard displays but uses the phrase 'non-desktop' instead. No functional changes.
[PATCH 1/3] drm: add connector info/property for non-desktop displays [v2]
From: Dave AirlieThis adds the infrastructure needed to quirk displays using edid and to mark them a non-desktop. A non-desktop display is one which shouldn't normally be included as a part of a desktop environment. This is meant to cover head mounted devices like HTC Vive. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_connector.c | 13 + drivers/gpu/drm/drm_edid.c | 8 ++-- include/drm/drm_connector.h | 5 + include/drm/drm_mode_config.h | 7 +++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc8934616..f9d3538c9588 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -234,6 +234,10 @@ int drm_connector_init(struct drm_device *dev, config->link_status_property, 0); + drm_object_attach_property(>base, + config->non_desktop_property, + 0); + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(>base, config->prop_crtc_id, 0); } @@ -811,6 +815,11 @@ int drm_connector_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.link_status_property = prop; + prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE, "non-desktop"); + if (!prop) + return -ENOMEM; + dev->mode_config.non_desktop_property = prop; + return 0; } @@ -1194,6 +1203,10 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + drm_object_property_set_value(>base, + dev->mode_config.non_desktop_property, + connector->display_info.non_desktop); + ret = drm_property_replace_global_blob(dev, >edid_blob_ptr, size, diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 00ddabfbf980..1e24d5d9d659 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_6BPC (1 << 10) /* Force 10bpc */ #define EDID_QUIRK_FORCE_10BPC (1 << 11) +/* Non desktop display (i.e. HMD) */ +#define EDID_QUIRK_NON_DESKTOP (1 << 12) struct detailed_mode_closure { struct drm_connector *connector; @@ -4393,7 +4395,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, } static void drm_add_display_info(struct drm_connector *connector, -struct edid *edid) +struct edid *edid, u32 quirks) { struct drm_display_info *info = >display_info; @@ -4407,6 +4409,8 @@ static void drm_add_display_info(struct drm_connector *connector, info->max_tmds_clock = 0; info->dvi_dual = false; + info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP); + if (edid->revision < 3) return; @@ -4627,7 +4631,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) * To avoid multiple parsing of same block, lets parse that map * from sink info, before parsing CEA modes. */ - drm_add_display_info(connector, edid); + drm_add_display_info(connector, edid, quirks); /* * EDID spec says modes should be preferred in this order: diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 7a7140543012..df9807a3caae 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -284,6 +284,11 @@ struct drm_display_info { * @hdmi: advance features of a HDMI sink. */ struct drm_hdmi_info hdmi; + + /** +* @non_desktop: Non desktop display (HMD). +*/ + bool non_desktop; }; int drm_display_info_set_bus_formats(struct drm_display_info *info, diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 0b4ac2ebc610..b21e827c5c78 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -728,6 +728,13 @@ struct drm_mode_config { */ struct drm_property *suggested_y_property; + /** +* @non_desktop_property: Optional connector property with a hint +* that device isn't a standard display, and the console/desktop, +* should not be displayed on it. +*/ + struct drm_property *non_desktop_property; + /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; -- 2.15.0.rc0
[PATCH 3/3] drm/edid: quirk HTC vive headset as non-desktop. [v2]
From: Dave AirlieThis uses the EDID info from my HTC Vive to mark it as non-desktop. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_edid.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1e24d5d9d659..2e8fb51282ef 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -159,6 +159,9 @@ static const struct edid_quirk { /* Rotel RSX-1058 forwards sink's EDID but only does HDMI 1.1*/ { "ETR", 13896, EDID_QUIRK_FORCE_8BPC }, + + /* HTC Vive VR Headset */ + { "HVR", 0xaa01, EDID_QUIRK_NON_DESKTOP }, }; /* -- 2.15.0.rc0
[PATCH 3/3] drm/edid: quirk HTC vive headset as non-desktop. [v2]
From: Dave Airlie This uses the EDID info from my HTC Vive to mark it as non-desktop. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_edid.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1e24d5d9d659..2e8fb51282ef 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -159,6 +159,9 @@ static const struct edid_quirk { /* Rotel RSX-1058 forwards sink's EDID but only does HDMI 1.1*/ { "ETR", 13896, EDID_QUIRK_FORCE_8BPC }, + + /* HTC Vive VR Headset */ + { "HVR", 0xaa01, EDID_QUIRK_NON_DESKTOP }, }; /* -- 2.15.0.rc0
[PATCH 1/3] drm: add connector info/property for non-desktop displays [v2]
From: Dave Airlie This adds the infrastructure needed to quirk displays using edid and to mark them a non-desktop. A non-desktop display is one which shouldn't normally be included as a part of a desktop environment. This is meant to cover head mounted devices like HTC Vive. v2: Change description from non-standard to non-desktop Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_connector.c | 13 + drivers/gpu/drm/drm_edid.c | 8 ++-- include/drm/drm_connector.h | 5 + include/drm/drm_mode_config.h | 7 +++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc8934616..f9d3538c9588 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -234,6 +234,10 @@ int drm_connector_init(struct drm_device *dev, config->link_status_property, 0); + drm_object_attach_property(>base, + config->non_desktop_property, + 0); + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(>base, config->prop_crtc_id, 0); } @@ -811,6 +815,11 @@ int drm_connector_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.link_status_property = prop; + prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE, "non-desktop"); + if (!prop) + return -ENOMEM; + dev->mode_config.non_desktop_property = prop; + return 0; } @@ -1194,6 +1203,10 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, if (edid) size = EDID_LENGTH * (1 + edid->extensions); + drm_object_property_set_value(>base, + dev->mode_config.non_desktop_property, + connector->display_info.non_desktop); + ret = drm_property_replace_global_blob(dev, >edid_blob_ptr, size, diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 00ddabfbf980..1e24d5d9d659 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_6BPC (1 << 10) /* Force 10bpc */ #define EDID_QUIRK_FORCE_10BPC (1 << 11) +/* Non desktop display (i.e. HMD) */ +#define EDID_QUIRK_NON_DESKTOP (1 << 12) struct detailed_mode_closure { struct drm_connector *connector; @@ -4393,7 +4395,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, } static void drm_add_display_info(struct drm_connector *connector, -struct edid *edid) +struct edid *edid, u32 quirks) { struct drm_display_info *info = >display_info; @@ -4407,6 +4409,8 @@ static void drm_add_display_info(struct drm_connector *connector, info->max_tmds_clock = 0; info->dvi_dual = false; + info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP); + if (edid->revision < 3) return; @@ -4627,7 +4631,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) * To avoid multiple parsing of same block, lets parse that map * from sink info, before parsing CEA modes. */ - drm_add_display_info(connector, edid); + drm_add_display_info(connector, edid, quirks); /* * EDID spec says modes should be preferred in this order: diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 7a7140543012..df9807a3caae 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -284,6 +284,11 @@ struct drm_display_info { * @hdmi: advance features of a HDMI sink. */ struct drm_hdmi_info hdmi; + + /** +* @non_desktop: Non desktop display (HMD). +*/ + bool non_desktop; }; int drm_display_info_set_bus_formats(struct drm_display_info *info, diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 0b4ac2ebc610..b21e827c5c78 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -728,6 +728,13 @@ struct drm_mode_config { */ struct drm_property *suggested_y_property; + /** +* @non_desktop_property: Optional connector property with a hint +* that device isn't a standard display, and the console/desktop, +* should not be displayed on it. +*/ + struct drm_property *non_desktop_property; + /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; -- 2.15.0.rc0
Re: [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
Sean Paulwrites: > nit: space before , Thanks. >> +/* Clone the lessor file to create a new file for us */ >> +DRM_DEBUG_LEASE("Allocating lease file\n"); >> +path_get(_file->f_path); > > Please forgive the stupid question, but where is this reference given > up? That's not a stupid question, it's a very subtle one which took me quite a while to sort out. Here's path_get: void path_get(const struct path *path) { mntget(path->mnt); dget(path->dentry); } So, getting a reference on a 'path' actually gets a reference on two of the things it points to. alloc_file is passed the path and doesn't take an additional reference on either of these fields, presumably because the normal path has the caller taking a reference while looking up the object and handing that reference off to alloc_file. In our case, we're creating a new file that refers to the same path as an existing one, so we need another reference. When the file is finally freed in __fput, the two references are dropped at the end of the function: static void __fput(struct file *file) { struct dentry *dentry = file->f_path.dentry; struct vfsmount *mnt = file->f_path.mnt; ... dput(dentry); mntput(mnt); } This was probably the twistiest part of creating a lease. All of the DRM stuff was trivial; getting the core kernel object reference counts right was a pain. >> +if (lessee->lessor == NULL) >> +/* owner can use all objects */ >> +object_idr = >dev->mode_config.crtc_idr; > > What about other types of objects? If I understand your question correctly, the answer is that 'crtc_idr' is misnamed -- it holds all of the mode setting objects. Thanks for your review, let me know if you have more questions! -- -keith signature.asc Description: PGP signature
Re: [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
Sean Paul writes: > nit: space before , Thanks. >> +/* Clone the lessor file to create a new file for us */ >> +DRM_DEBUG_LEASE("Allocating lease file\n"); >> +path_get(_file->f_path); > > Please forgive the stupid question, but where is this reference given > up? That's not a stupid question, it's a very subtle one which took me quite a while to sort out. Here's path_get: void path_get(const struct path *path) { mntget(path->mnt); dget(path->dentry); } So, getting a reference on a 'path' actually gets a reference on two of the things it points to. alloc_file is passed the path and doesn't take an additional reference on either of these fields, presumably because the normal path has the caller taking a reference while looking up the object and handing that reference off to alloc_file. In our case, we're creating a new file that refers to the same path as an existing one, so we need another reference. When the file is finally freed in __fput, the two references are dropped at the end of the function: static void __fput(struct file *file) { struct dentry *dentry = file->f_path.dentry; struct vfsmount *mnt = file->f_path.mnt; ... dput(dentry); mntput(mnt); } This was probably the twistiest part of creating a lease. All of the DRM stuff was trivial; getting the core kernel object reference counts right was a pain. >> +if (lessee->lessor == NULL) >> +/* owner can use all objects */ >> +object_idr = >dev->mode_config.crtc_idr; > > What about other types of objects? If I understand your question correctly, the answer is that 'crtc_idr' is misnamed -- it holds all of the mode setting objects. Thanks for your review, let me know if you have more questions! -- -keith signature.asc Description: PGP signature
Re: [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
Sean Paul <seanp...@chromium.org> writes: > With these nits fixed, > Reviewed-by: Sean Paul <seanp...@chromium.org> Like this? From 0aa52dd5a0873831c79c14942075354c041e5bed Mon Sep 17 00:00:00 2001 From: Keith Packard <kei...@keithp.com> Date: Mon, 16 Oct 2017 13:41:20 -0700 Subject: [PATCH] drm: Mark functions requiring idr_mutex. Add lockdep to _drm_lease_revoke Reasonable suggestions by Sean Paul. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_lease.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 88c213f9c4ab..20694c77a2de 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -58,7 +58,9 @@ _drm_find_lessee(struct drm_master *master, int lessee_id) } /** - * _drm_lease_held_master - check to see if an object is leased (or owned) by master + * _drm_lease_held_master - check to see if an object is leased (or + * owned) by master (idr_mutex held) + * * @master: the master to check the lease status of * @id: the id to check * @@ -77,7 +79,7 @@ static int _drm_lease_held_master(struct drm_master *master, int id) } /** - * _drm_has_leased - check to see if an object has been leased + * _drm_has_leased - check to see if an object has been leased (idr mutex held) * @master: the master to check the lease status of * @id: the id to check * @@ -300,8 +302,8 @@ void drm_lease_destroy(struct drm_master *master) } /** - * _drm_lease_revoke - revoke access to all leased objects - * @master: the master losing its lease + * _drm_lease_revoke - revoke access to all leased objects (idr_mutex held) + * @top: the master losing its lease */ void _drm_lease_revoke(struct drm_master *top) @@ -310,6 +312,7 @@ void _drm_lease_revoke(struct drm_master *top) void *entry; struct drm_master *master = top; + lockdep_assert_held(>dev->mode_config.idr_mutex); /* * Walk the tree starting at 'top' emptying all leases. Because * the tree is fully connected, we can do this without recursing -- 2.15.0.rc0 -- -keith signature.asc Description: PGP signature
Re: [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
Sean Paul writes: > With these nits fixed, > Reviewed-by: Sean Paul Like this? From 0aa52dd5a0873831c79c14942075354c041e5bed Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 16 Oct 2017 13:41:20 -0700 Subject: [PATCH] drm: Mark functions requiring idr_mutex. Add lockdep to _drm_lease_revoke Reasonable suggestions by Sean Paul. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_lease.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 88c213f9c4ab..20694c77a2de 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -58,7 +58,9 @@ _drm_find_lessee(struct drm_master *master, int lessee_id) } /** - * _drm_lease_held_master - check to see if an object is leased (or owned) by master + * _drm_lease_held_master - check to see if an object is leased (or + * owned) by master (idr_mutex held) + * * @master: the master to check the lease status of * @id: the id to check * @@ -77,7 +79,7 @@ static int _drm_lease_held_master(struct drm_master *master, int id) } /** - * _drm_has_leased - check to see if an object has been leased + * _drm_has_leased - check to see if an object has been leased (idr mutex held) * @master: the master to check the lease status of * @id: the id to check * @@ -300,8 +302,8 @@ void drm_lease_destroy(struct drm_master *master) } /** - * _drm_lease_revoke - revoke access to all leased objects - * @master: the master losing its lease + * _drm_lease_revoke - revoke access to all leased objects (idr_mutex held) + * @top: the master losing its lease */ void _drm_lease_revoke(struct drm_master *top) @@ -310,6 +312,7 @@ void _drm_lease_revoke(struct drm_master *top) void *entry; struct drm_master *master = top; + lockdep_assert_held(>dev->mode_config.idr_mutex); /* * Walk the tree starting at 'top' emptying all leases. Because * the tree is fully connected, we can do this without recursing -- 2.15.0.rc0 -- -keith signature.asc Description: PGP signature
Re: [PATCH 0/5]: drm: Add drm mode object leases
Daniel Vetterwrites: > A bit lacking time to do an in-depth review, but all my major concerns > seem to be addressed. On the series. Thanks much. I sent out a note over the weekend about how we can make the Vulkan API work without the permissions change in the kernel. We need to provide access to a DRM master and can choose to either use the X server's or pass a master FD in explicitly. -- -keith signature.asc Description: PGP signature
Re: [PATCH 0/5]: drm: Add drm mode object leases
Daniel Vetter writes: > A bit lacking time to do an in-depth review, but all my major concerns > seem to be addressed. On the series. Thanks much. I sent out a note over the weekend about how we can make the Vulkan API work without the permissions change in the kernel. We need to provide access to a DRM master and can choose to either use the X server's or pass a master FD in explicitly. -- -keith signature.asc Description: PGP signature
Re: [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
Sean Paulwrites: > Sphinx won't pick these up, and will issue warnings. Please update to @ > instead of \param > > If you want to try it out: > make htmldocs Yeah, I was attempting to emulate the existing style. I suggest that a general cleanup to fix the docstrings should be in a separate patch and cover a file at at time so that we can reasonably test the results. I don't have a strong opinion on what format we should use before that happens. > Otherwise, > > Reviewed-by: Sean Paul Thanks for your review. -- -keith signature.asc Description: PGP signature
Re: [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
Sean Paul writes: > Sphinx won't pick these up, and will issue warnings. Please update to @ > instead of \param > > If you want to try it out: > make htmldocs Yeah, I was attempting to emulate the existing style. I suggest that a general cleanup to fix the docstrings should be in a separate patch and cover a file at at time so that we can reasonably test the results. I don't have a strong opinion on what format we should use before that happens. > Otherwise, > > Reviewed-by: Sean Paul Thanks for your review. -- -keith signature.asc Description: PGP signature
[PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
This provides new data structures to hold "lease" information about drm mode setting objects, and provides for creating new drm_masters which have access to a subset of the available drm resources. An 'owner' is a drm_master which is not leasing the objects from another drm_master, and hence 'owns' them. A 'lessee' is a drm_master which is leasing objects from some other drm_master. Each lessee holds the set of objects which it is leasing from the lessor. A 'lessor' is a drm_master which is leasing objects to another drm_master. This is the same as the owner in the current code. The set of objects any drm_master 'controls' is limited to the set of objects it leases (for lessees) or all objects (for owners). Objects not controlled by a drm_master cannot be modified through the various state manipulating ioctls, and any state reported back to user space will be edited to make them appear idle and/or unusable. For instance, connectors always report 'disconnected', while encoders report no possible crtcs or clones. The full list of lessees leasing objects from an owner (either directly, or indirectly through another lessee), can be searched from an idr in the drm_master of the owner. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: * Sub-leasing has been disabled. * BUG_ON for lock checking replaced with lockdep_assert_held * 'change' ioctl has been removed. * Leased objects can always be controlled by the lessor; the 'mask_lease' flag has been removed * Checking for leased status has been simplified, replacing the drm_lease_check function with drm_lease_held. Changes in v3, some suggested by Dave Airlie <airl...@gmail.com> * Add revocation. This allows leases to be effectively revoked by removing all of the objects they have access to. The lease itself hangs around as it's hanging off a file. * Free the leases IDR when the master is destroyed * _drm_lease_held should look at lessees, not lessor * Allow non-master files to check for lease status Changes in v4, suggested by Dave Airlie <airl...@gmail.com> * Formatting and whitespace changes Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_auth.c| 29 +++- drivers/gpu/drm/drm_lease.c | 339 ++ include/drm/drm_auth.h| 20 +++ include/drm/drm_lease.h | 36 + include/drm/drm_mode_object.h | 1 + 6 files changed, 425 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_lease.c create mode 100644 include/drm/drm_lease.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a3fdc5a68dff..81ff79336623 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -17,7 +17,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ - drm_syncobj.o + drm_syncobj.o drm_lease.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 7ff697389d74..541177c71d51 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -31,6 +31,7 @@ #include #include "drm_internal.h" #include "drm_legacy.h" +#include /** * DOC: master and authentication @@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data, return file ? 0 : -EINVAL; } -static struct drm_master *drm_master_create(struct drm_device *dev) +struct drm_master *drm_master_create(struct drm_device *dev) { struct drm_master *master; @@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev) idr_init(>magic_map); master->dev = dev; + /* initialize the tree of output resource lessees */ + master->lessor = NULL; + master->lessee_id = 0; + INIT_LIST_HEAD(>lessees); + INIT_LIST_HEAD(>lessee_list); + idr_init(>leases); + idr_init(>lessee_idr); + return master; } @@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; } + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(>master_mutex); @@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv) if (dev->master == file_priv->master) drm_drop_master(dev, file_priv); ou
[PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
This provides new data structures to hold "lease" information about drm mode setting objects, and provides for creating new drm_masters which have access to a subset of the available drm resources. An 'owner' is a drm_master which is not leasing the objects from another drm_master, and hence 'owns' them. A 'lessee' is a drm_master which is leasing objects from some other drm_master. Each lessee holds the set of objects which it is leasing from the lessor. A 'lessor' is a drm_master which is leasing objects to another drm_master. This is the same as the owner in the current code. The set of objects any drm_master 'controls' is limited to the set of objects it leases (for lessees) or all objects (for owners). Objects not controlled by a drm_master cannot be modified through the various state manipulating ioctls, and any state reported back to user space will be edited to make them appear idle and/or unusable. For instance, connectors always report 'disconnected', while encoders report no possible crtcs or clones. The full list of lessees leasing objects from an owner (either directly, or indirectly through another lessee), can be searched from an idr in the drm_master of the owner. Changes for v2 as suggested by Daniel Vetter : * Sub-leasing has been disabled. * BUG_ON for lock checking replaced with lockdep_assert_held * 'change' ioctl has been removed. * Leased objects can always be controlled by the lessor; the 'mask_lease' flag has been removed * Checking for leased status has been simplified, replacing the drm_lease_check function with drm_lease_held. Changes in v3, some suggested by Dave Airlie * Add revocation. This allows leases to be effectively revoked by removing all of the objects they have access to. The lease itself hangs around as it's hanging off a file. * Free the leases IDR when the master is destroyed * _drm_lease_held should look at lessees, not lessor * Allow non-master files to check for lease status Changes in v4, suggested by Dave Airlie * Formatting and whitespace changes Signed-off-by: Keith Packard --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_auth.c| 29 +++- drivers/gpu/drm/drm_lease.c | 339 ++ include/drm/drm_auth.h| 20 +++ include/drm/drm_lease.h | 36 + include/drm/drm_mode_object.h | 1 + 6 files changed, 425 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_lease.c create mode 100644 include/drm/drm_lease.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a3fdc5a68dff..81ff79336623 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -17,7 +17,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ - drm_syncobj.o + drm_syncobj.o drm_lease.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 7ff697389d74..541177c71d51 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -31,6 +31,7 @@ #include #include "drm_internal.h" #include "drm_legacy.h" +#include /** * DOC: master and authentication @@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data, return file ? 0 : -EINVAL; } -static struct drm_master *drm_master_create(struct drm_device *dev) +struct drm_master *drm_master_create(struct drm_device *dev) { struct drm_master *master; @@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev) idr_init(>magic_map); master->dev = dev; + /* initialize the tree of output resource lessees */ + master->lessor = NULL; + master->lessee_id = 0; + INIT_LIST_HEAD(>lessees); + INIT_LIST_HEAD(>lessee_list); + idr_init(>leases); + idr_init(>lessee_idr); + return master; } @@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; } + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(>master_mutex); @@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv) if (dev->master == file_priv->master) drm_drop_master(dev, file_priv); out: + if (file_priv->is_master) { + /* Revoke any leases held by this or lessees, but only if +
[PATCH 2/5] drm: Add new LEASE debug level
Separate out lease debugging from the core. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_drv.c | 3 ++- include/drm/drmP.h| 4 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c0292e5d7281..a934fd5e7e55 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" -"\t\tBit 5 (0x20) will enable VBL messages (vblank code)"); +"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" +"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)"); module_param_named(debug, drm_debug, int, 0600); static DEFINE_SPINLOCK(drm_minor_lock); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 7277783a4ff0..59be1232d005 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -136,6 +136,7 @@ struct pci_controller; #define DRM_UT_ATOMIC 0x10 #define DRM_UT_VBL 0x20 #define DRM_UT_STATE 0x40 +#define DRM_UT_LEASE 0x80 /***/ /** \name DRM template customization defaults */ @@ -250,6 +251,9 @@ struct pci_controller; #define DRM_DEBUG_VBL(fmt, ...)\ drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__) +#define DRM_DEBUG_LEASE(fmt, ...) \ + drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__) + #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)\ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ -- 2.15.0.rc0
[PATCH 0/5]: drm: Add drm mode object leases
New since last time: * Don't lease encoders * Do lease planes * Automatically lease primary and cursor planes for apps which don't set universal_planes * Restrict leases to only contain objects which are actually leasable (connectors, crtcs and planes) * Drop the patch which changes permissions on get resources ioctls
[PATCH 2/5] drm: Add new LEASE debug level
Separate out lease debugging from the core. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_drv.c | 3 ++- include/drm/drmP.h| 4 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c0292e5d7281..a934fd5e7e55 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" -"\t\tBit 5 (0x20) will enable VBL messages (vblank code)"); +"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" +"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)"); module_param_named(debug, drm_debug, int, 0600); static DEFINE_SPINLOCK(drm_minor_lock); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 7277783a4ff0..59be1232d005 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -136,6 +136,7 @@ struct pci_controller; #define DRM_UT_ATOMIC 0x10 #define DRM_UT_VBL 0x20 #define DRM_UT_STATE 0x40 +#define DRM_UT_LEASE 0x80 /***/ /** \name DRM template customization defaults */ @@ -250,6 +251,9 @@ struct pci_controller; #define DRM_DEBUG_VBL(fmt, ...)\ drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__) +#define DRM_DEBUG_LEASE(fmt, ...) \ + drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__) + #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)\ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ -- 2.15.0.rc0
[PATCH 0/5]: drm: Add drm mode object leases
New since last time: * Don't lease encoders * Do lease planes * Automatically lease primary and cursor planes for apps which don't set universal_planes * Restrict leases to only contain objects which are actually leasable (connectors, crtcs and planes) * Drop the patch which changes permissions on get resources ioctls
[PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3]
Attempts to modify un-leased objects are rejected with an error. Information returned about unleased objects is modified to make them appear unusable and/or disconnected. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: * With the change in the __drm_mode_object_find API to pass the file_priv along, we can now centralize most of the lease-based access checks in that function. * A few places skip that API and require in-line checks. Changes for v3 provided by Dave Airlie <airl...@redhat.com> * remove support for leasing encoders. * add support for leasing planes. Signed-off-by: Keith Packard <kei...@keithp.com> Signed-off-by: Dave Airlie <airl...@redhat.com> --- drivers/gpu/drm/drm_auth.c| 2 +- drivers/gpu/drm/drm_encoder.c | 5 +++-- drivers/gpu/drm/drm_mode_config.c | 22 +- drivers/gpu/drm/drm_mode_object.c | 22 ++ drivers/gpu/drm/drm_plane.c | 18 +++--- drivers/gpu/drm/drm_vblank.c | 18 -- include/drm/drm_lease.h | 2 -- 7 files changed, 66 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 541177c71d51..bab26b477738 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv) */ bool drm_is_current_master(struct drm_file *fpriv) { - return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } EXPORT_SYMBOL(drm_is_current_master); diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 43f644844b83..59e0ebe733f8 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc) + if (crtc && drm_lease_held(file_priv, crtc->base.id)) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; @@ -234,7 +234,8 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; - enc_resp->possible_crtcs = encoder->possible_crtcs; + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv, + encoder->possible_crtcs); enc_resp->possible_clones = encoder->possible_clones; return 0; diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 919e78d45ab0..cda8bfab6d3b 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -122,10 +122,12 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr); drm_for_each_crtc(crtc, dev) { - if (count < card_res->count_crtcs && - put_user(crtc->base.id, crtc_id + count)) - return -EFAULT; - count++; + if (drm_lease_held(file_priv, crtc->base.id)) { + if (count < card_res->count_crtcs && + put_user(crtc->base.id, crtc_id + count)) + return -EFAULT; + count++; + } } card_res->count_crtcs = count; @@ -143,12 +145,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; connector_id = u64_to_user_ptr(card_res->connector_id_ptr); drm_for_each_connector_iter(connector, _iter) { - if (count < card_res->count_connectors && - put_user(connector->base.id, connector_id + count)) { - drm_connector_list_iter_end(_iter); - return -EFAULT; + if (drm_lease_held(file_priv, connector->base.id)) { + if (count < card_res->count_connectors && + put_user(connector->base.id, connector_id + count)) { + drm_connector_list_iter_end(_iter); + return -EFAULT; + } + count++; } - count++; } card_res->count_connectors = count; drm_connector_list_iter_end(_iter); diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 240a05d91a53..d1599f36b605 100644 --- a/drivers/gpu/drm/drm_
[PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3]
Attempts to modify un-leased objects are rejected with an error. Information returned about unleased objects is modified to make them appear unusable and/or disconnected. Changes for v2 as suggested by Daniel Vetter : * With the change in the __drm_mode_object_find API to pass the file_priv along, we can now centralize most of the lease-based access checks in that function. * A few places skip that API and require in-line checks. Changes for v3 provided by Dave Airlie * remove support for leasing encoders. * add support for leasing planes. Signed-off-by: Keith Packard Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_auth.c| 2 +- drivers/gpu/drm/drm_encoder.c | 5 +++-- drivers/gpu/drm/drm_mode_config.c | 22 +- drivers/gpu/drm/drm_mode_object.c | 22 ++ drivers/gpu/drm/drm_plane.c | 18 +++--- drivers/gpu/drm/drm_vblank.c | 18 -- include/drm/drm_lease.h | 2 -- 7 files changed, 66 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 541177c71d51..bab26b477738 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv) */ bool drm_is_current_master(struct drm_file *fpriv) { - return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } EXPORT_SYMBOL(drm_is_current_master); diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 43f644844b83..59e0ebe733f8 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc) + if (crtc && drm_lease_held(file_priv, crtc->base.id)) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; @@ -234,7 +234,8 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; - enc_resp->possible_crtcs = encoder->possible_crtcs; + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv, + encoder->possible_crtcs); enc_resp->possible_clones = encoder->possible_clones; return 0; diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 919e78d45ab0..cda8bfab6d3b 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -122,10 +122,12 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr); drm_for_each_crtc(crtc, dev) { - if (count < card_res->count_crtcs && - put_user(crtc->base.id, crtc_id + count)) - return -EFAULT; - count++; + if (drm_lease_held(file_priv, crtc->base.id)) { + if (count < card_res->count_crtcs && + put_user(crtc->base.id, crtc_id + count)) + return -EFAULT; + count++; + } } card_res->count_crtcs = count; @@ -143,12 +145,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; connector_id = u64_to_user_ptr(card_res->connector_id_ptr); drm_for_each_connector_iter(connector, _iter) { - if (count < card_res->count_connectors && - put_user(connector->base.id, connector_id + count)) { - drm_connector_list_iter_end(_iter); - return -EFAULT; + if (drm_lease_held(file_priv, connector->base.id)) { + if (count < card_res->count_connectors && + put_user(connector->base.id, connector_id + count)) { + drm_connector_list_iter_end(_iter); + return -EFAULT; + } + count++; } - count++; } card_res->count_connectors = count; drm_connector_list_iter_end(_iter); diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 240a05d91a53..d1599f36b605 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -104,6 +104,25 @@ void drm_mode_object_unregister(struc
[PATCH 1/5] drm/plane: drop num_overlay_planes (v2)
From: Dave AirlieIn order to implement plane leasing we need to count things, just make the code consistent with the counting code currently used for counting crtcs/encoders/connectors and drop the need for num_overlay_planes. v2: don't forget to assign plane_ptr. (keithp) Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_mode_config.c | 1 - drivers/gpu/drm/drm_plane.c | 46 ++- include/drm/drm_mode_config.h | 13 --- 3 files changed, 16 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 74f6ff5df656..919e78d45ab0 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -385,7 +385,6 @@ void drm_mode_config_init(struct drm_device *dev) dev->mode_config.num_connector = 0; dev->mode_config.num_crtc = 0; dev->mode_config.num_encoder = 0; - dev->mode_config.num_overlay_plane = 0; dev->mode_config.num_total_plane = 0; } EXPORT_SYMBOL(drm_mode_config_init); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 574fd1475214..c10869bad729 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -241,8 +241,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, list_add_tail(>head, >plane_list); plane->index = config->num_total_plane++; - if (plane->type == DRM_PLANE_TYPE_OVERLAY) - config->num_overlay_plane++; drm_object_attach_property(>base, config->plane_type_property, @@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane) list_del(>head); dev->mode_config.num_total_plane--; - if (plane->type == DRM_PLANE_TYPE_OVERLAY) - dev->mode_config.num_overlay_plane--; WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); if (plane->state && plane->funcs->atomic_destroy_state) @@ -462,43 +458,33 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data, struct drm_mode_config *config; struct drm_plane *plane; uint32_t __user *plane_ptr; - int copied = 0; - unsigned num_planes; + int count = 0; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; config = >mode_config; - - if (file_priv->universal_planes) - num_planes = config->num_total_plane; - else - num_planes = config->num_overlay_plane; + plane_ptr = u64_to_user_ptr(plane_resp->plane_id_ptr); /* * This ioctl is called twice, once to determine how much space is * needed, and the 2nd time to fill it. */ - if (num_planes && - (plane_resp->count_planes >= num_planes)) { - plane_ptr = (uint32_t __user *)(unsigned long)plane_resp->plane_id_ptr; - - /* Plane lists are invariant, no locking needed. */ - drm_for_each_plane(plane, dev) { - /* -* Unless userspace set the 'universal planes' -* capability bit, only advertise overlays. -*/ - if (plane->type != DRM_PLANE_TYPE_OVERLAY && - !file_priv->universal_planes) - continue; - - if (put_user(plane->base.id, plane_ptr + copied)) - return -EFAULT; - copied++; - } + drm_for_each_plane(plane, dev) { + /* +* Unless userspace set the 'universal planes' +* capability bit, only advertise overlays. +*/ + if (plane->type != DRM_PLANE_TYPE_OVERLAY && + !file_priv->universal_planes) + continue; + + if (count < config->num_total_plane && + put_user(plane->base.id, plane_ptr + count)) + return -EFAULT; + count++; } - plane_resp->count_planes = num_planes; + plane_resp->count_planes = count; return 0; } diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1b37368416c8..0b4ac2ebc610 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -429,19 +429,6 @@ struct drm_mode_config { */ struct list_head encoder_list; - /** -* @num_overlay_plane: -* -* Number of overlay planes on this device, excluding primary and cursor -* planes. -* -* Track number of overlay planes separately from number of total -* planes. By default we only advertise overlay planes to userspace; if -* userspace sets the "universal plane" capability bit,
[PATCH 1/5] drm/plane: drop num_overlay_planes (v2)
From: Dave Airlie In order to implement plane leasing we need to count things, just make the code consistent with the counting code currently used for counting crtcs/encoders/connectors and drop the need for num_overlay_planes. v2: don't forget to assign plane_ptr. (keithp) Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_mode_config.c | 1 - drivers/gpu/drm/drm_plane.c | 46 ++- include/drm/drm_mode_config.h | 13 --- 3 files changed, 16 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 74f6ff5df656..919e78d45ab0 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -385,7 +385,6 @@ void drm_mode_config_init(struct drm_device *dev) dev->mode_config.num_connector = 0; dev->mode_config.num_crtc = 0; dev->mode_config.num_encoder = 0; - dev->mode_config.num_overlay_plane = 0; dev->mode_config.num_total_plane = 0; } EXPORT_SYMBOL(drm_mode_config_init); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 574fd1475214..c10869bad729 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -241,8 +241,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, list_add_tail(>head, >plane_list); plane->index = config->num_total_plane++; - if (plane->type == DRM_PLANE_TYPE_OVERLAY) - config->num_overlay_plane++; drm_object_attach_property(>base, config->plane_type_property, @@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane) list_del(>head); dev->mode_config.num_total_plane--; - if (plane->type == DRM_PLANE_TYPE_OVERLAY) - dev->mode_config.num_overlay_plane--; WARN_ON(plane->state && !plane->funcs->atomic_destroy_state); if (plane->state && plane->funcs->atomic_destroy_state) @@ -462,43 +458,33 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data, struct drm_mode_config *config; struct drm_plane *plane; uint32_t __user *plane_ptr; - int copied = 0; - unsigned num_planes; + int count = 0; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; config = >mode_config; - - if (file_priv->universal_planes) - num_planes = config->num_total_plane; - else - num_planes = config->num_overlay_plane; + plane_ptr = u64_to_user_ptr(plane_resp->plane_id_ptr); /* * This ioctl is called twice, once to determine how much space is * needed, and the 2nd time to fill it. */ - if (num_planes && - (plane_resp->count_planes >= num_planes)) { - plane_ptr = (uint32_t __user *)(unsigned long)plane_resp->plane_id_ptr; - - /* Plane lists are invariant, no locking needed. */ - drm_for_each_plane(plane, dev) { - /* -* Unless userspace set the 'universal planes' -* capability bit, only advertise overlays. -*/ - if (plane->type != DRM_PLANE_TYPE_OVERLAY && - !file_priv->universal_planes) - continue; - - if (put_user(plane->base.id, plane_ptr + copied)) - return -EFAULT; - copied++; - } + drm_for_each_plane(plane, dev) { + /* +* Unless userspace set the 'universal planes' +* capability bit, only advertise overlays. +*/ + if (plane->type != DRM_PLANE_TYPE_OVERLAY && + !file_priv->universal_planes) + continue; + + if (count < config->num_total_plane && + put_user(plane->base.id, plane_ptr + count)) + return -EFAULT; + count++; } - plane_resp->count_planes = num_planes; + plane_resp->count_planes = count; return 0; } diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1b37368416c8..0b4ac2ebc610 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -429,19 +429,6 @@ struct drm_mode_config { */ struct list_head encoder_list; - /** -* @num_overlay_plane: -* -* Number of overlay planes on this device, excluding primary and cursor -* planes. -* -* Track number of overlay planes separately from number of total -* planes. By default we only advertise overlay planes to userspace; if -* userspace sets the "universal plane" capability bit, we'll go ahead -* and expose all
[PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
drm_mode_create_lease Creates a lease for a list of drm mode objects, returning an fd for the new drm_master and a 64-bit identifier for the lessee drm_mode_list_lesees List the identifiers of the lessees for a master file drm_mode_get_lease List the leased objects for a master file drm_mode_revoke_lease Erase the set of objects managed by a lease. This should suffice to at least create and query leases. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: * query ioctls only query the master associated with the provided file. * 'mask_lease' value has been removed * change ioctl has been removed. Changes for v3 suggested in part by Dave Airlie <airl...@gmail.com> * Add revoke ioctl. Changes for v4 suggested by Dave Airlie <airl...@gmail.com> * Expand on the comment about the magic use of _lease_idr_object * Pad lease ioctl structures to align on 64-bit boundaries Changes for v5 suggested by Dave Airlie <airl...@gmail.com> * Check for non-negative object_id in create_lease to avoid debug output from the kernel. Changes for v5 provided by Dave Airlie <airl...@gmail.com> * For non-universal planes add primary/cursor planes to lease If we aren't exposing universal planes to this userspace client, and it requests a lease on a crtc, we should implicitly export the primary and cursor planes for the crtc. If the lessee doesn't request universal planes, it will just see the crtc, but if it does request them it will then see the plane objects as well. This also moves the object look ups earlier as a side effect, so we'd exit the ioctl quicker for non-existant objects. * Restrict leases to crtc/connector/planes. This only allows leasing for objects we wish to allow. Signed-off-by: Keith Packard <kei...@keithp.com> Signed-off-by: Dave Airlie <airl...@redhat.com> --- drivers/gpu/drm/drm_ioctl.c | 4 + drivers/gpu/drm/drm_lease.c | 318 ++ drivers/gpu/drm/drm_mode_object.c | 5 +- include/drm/drm_lease.h | 12 ++ include/drm/drm_mode_object.h | 2 + include/uapi/drm/drm.h| 5 + include/uapi/drm/drm_mode.h | 66 7 files changed, 410 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9c435a4c0c82..4aafe4802099 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 6c3f2445254c..88c213f9c4ab 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -23,6 +23,8 @@ #define drm_for_each_lessee(lessee, lessor) \ list_for_each_entry((lessee), &(lessor)->lessees, lessee_list) +static uint64_t drm_lease_idr_object; + /** * drm_lease_owner - return ancestor owner drm_master * @master: drm_master somewhere within tree of lessees and lessors @@ -337,3 +339,319 @@ void _drm_lease_revoke(struct drm_master *top) } } } + +/** + * drm_mode_create_lease_ioctl - create a new lease + * @dev: the drm device + * @data: pointer to struct drm_mode_create_lease + * @file_priv: the file being manipulated + * + * The master associated with the specified file will have a lease + * created containing the objects specified in the ioctl structure. + * A file descriptor will be allocated for that and returned to the + * application. + */ +int drm_mode_create_lease_ioctl(struct drm_device *dev, + void *data, struct drm_file *lessor_priv) +{ + struct drm_mode_create_lease *cl = data; + size_t object_count; + size_t o; + int ret = 0; + struct idr leases; + struct drm_master *lessor = lessor_priv->master; + struct drm_master *lessee = NULL; + struct file *lessee_file = NULL; + struct file *lessor_file = lessor_priv->filp; + struct drm_file *lessee_priv; + int fd = -1; + + /* Do not allow sub-leases */ +
[PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
drm_mode_create_lease Creates a lease for a list of drm mode objects, returning an fd for the new drm_master and a 64-bit identifier for the lessee drm_mode_list_lesees List the identifiers of the lessees for a master file drm_mode_get_lease List the leased objects for a master file drm_mode_revoke_lease Erase the set of objects managed by a lease. This should suffice to at least create and query leases. Changes for v2 as suggested by Daniel Vetter : * query ioctls only query the master associated with the provided file. * 'mask_lease' value has been removed * change ioctl has been removed. Changes for v3 suggested in part by Dave Airlie * Add revoke ioctl. Changes for v4 suggested by Dave Airlie * Expand on the comment about the magic use of _lease_idr_object * Pad lease ioctl structures to align on 64-bit boundaries Changes for v5 suggested by Dave Airlie * Check for non-negative object_id in create_lease to avoid debug output from the kernel. Changes for v5 provided by Dave Airlie * For non-universal planes add primary/cursor planes to lease If we aren't exposing universal planes to this userspace client, and it requests a lease on a crtc, we should implicitly export the primary and cursor planes for the crtc. If the lessee doesn't request universal planes, it will just see the crtc, but if it does request them it will then see the plane objects as well. This also moves the object look ups earlier as a side effect, so we'd exit the ioctl quicker for non-existant objects. * Restrict leases to crtc/connector/planes. This only allows leasing for objects we wish to allow. Signed-off-by: Keith Packard Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_ioctl.c | 4 + drivers/gpu/drm/drm_lease.c | 318 ++ drivers/gpu/drm/drm_mode_object.c | 5 +- include/drm/drm_lease.h | 12 ++ include/drm/drm_mode_object.h | 2 + include/uapi/drm/drm.h| 5 + include/uapi/drm/drm_mode.h | 66 7 files changed, 410 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9c435a4c0c82..4aafe4802099 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 6c3f2445254c..88c213f9c4ab 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -23,6 +23,8 @@ #define drm_for_each_lessee(lessee, lessor) \ list_for_each_entry((lessee), &(lessor)->lessees, lessee_list) +static uint64_t drm_lease_idr_object; + /** * drm_lease_owner - return ancestor owner drm_master * @master: drm_master somewhere within tree of lessees and lessors @@ -337,3 +339,319 @@ void _drm_lease_revoke(struct drm_master *top) } } } + +/** + * drm_mode_create_lease_ioctl - create a new lease + * @dev: the drm device + * @data: pointer to struct drm_mode_create_lease + * @file_priv: the file being manipulated + * + * The master associated with the specified file will have a lease + * created containing the objects specified in the ioctl structure. + * A file descriptor will be allocated for that and returned to the + * application. + */ +int drm_mode_create_lease_ioctl(struct drm_device *dev, + void *data, struct drm_file *lessor_priv) +{ + struct drm_mode_create_lease *cl = data; + size_t object_count; + size_t o; + int ret = 0; + struct idr leases; + struct drm_master *lessor = lessor_priv->master; + struct drm_master *lessee = NULL; + struct file *lessee_file = NULL; + struct file *lessor_file = lessor_priv->filp; + struct drm_file *lessee_priv; + int fd = -1; + + /* Do not allow sub-leases */ + if (lessor->lessor) + return -EINVAL; + + object_count = cl->object_count; + idr_init(); + + /* Allocate a file descriptor for the
[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]
Place drm_event_vblank in a new union that includes that and a bare drm_event structure. This will allow new members of that union to be added in the future without changing code related to the existing vbl event type. Assignments to the crtc_id field are now done when the event is allocated, rather than when delievered. This way, delivery doesn't need to have the crtc ID available. v2: * Remove 'dev' argument from create_vblank_event It wasn't being used anyways, and if we need it in the future, we can always get it from crtc->dev. * Check for MODESETTING before looking for crtc in queue_vblank_event UMS drivers will oops if we try to get a crtc, so make sure we're modesetting before we try to find a crtc_id to fill into the event. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_atomic.c | 7 +++--- drivers/gpu/drm/drm_plane.c | 2 +- drivers/gpu/drm/drm_vblank.c | 41 +--- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- include/drm/drm_vblank.h | 8 ++- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 268969fecee7..1e2021bed265 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor) */ static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, uint64_t user_data) + struct drm_crtc *crtc, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; @@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = user_data; + e->event.vbl.crtc_id = crtc->base.id; + e->event.vbl.user_data = user_data; return e; } @@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) { struct drm_pending_vblank_event *e; - e = create_vblank_event(dev, arg->user_data); + e = create_vblank_event(crtc, arg->user_data); if (!e) return -ENOMEM; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 6af02c7b5da3..574fd1475214 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, } e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = page_flip->user_data; + e->event.vbl.user_data = page_flip->user_data; ret = drm_event_reserve_init(dev, file_priv, >base, >event.base); if (ret) { kfree(e); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 47460589d4cd..d250606cc771 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -804,20 +804,23 @@ static void send_vblank_event(struct drm_device *dev, struct drm_pending_vblank_event *e, u64 seq, ktime_t now) { - struct timespec64 tv = ktime_to_timespec64(now); - - e->event.sequence = seq; - /* -* e->event is a user space structure, with hardcoded unsigned -* 32-bit seconds/microseconds. This is safe as we always use -* monotonic timestamps since linux-4.15 -*/ - e->event.tv_sec = tv.tv_sec; - e->event.tv_usec = tv.tv_nsec / 1000; - - trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, -e->event.sequence); + struct timespec64 tv; + switch (e->event.base.type) { + case DRM_EVENT_VBLANK: + case DRM_EVENT_FLIP_COMPLETE: + tv = ktime_to_timespec64(now); + e->event.vbl.sequence = seq; + /* +* e->event is a user space structure, with hardcoded unsigned +* 32-bit seconds/microseconds. This is safe as we always use +* monotonic timestamps since linux-4.15 +*/ + e->event.vbl.tv_sec = tv.tv_sec; + e->event.vbl.tv_usec = tv.tv_nsec / 1000; + break; + } + trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); } @@ -869,7 +872,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, e->pipe = pi
[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]
Place drm_event_vblank in a new union that includes that and a bare drm_event structure. This will allow new members of that union to be added in the future without changing code related to the existing vbl event type. Assignments to the crtc_id field are now done when the event is allocated, rather than when delievered. This way, delivery doesn't need to have the crtc ID available. v2: * Remove 'dev' argument from create_vblank_event It wasn't being used anyways, and if we need it in the future, we can always get it from crtc->dev. * Check for MODESETTING before looking for crtc in queue_vblank_event UMS drivers will oops if we try to get a crtc, so make sure we're modesetting before we try to find a crtc_id to fill into the event. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_atomic.c | 7 +++--- drivers/gpu/drm/drm_plane.c | 2 +- drivers/gpu/drm/drm_vblank.c | 41 +--- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- include/drm/drm_vblank.h | 8 ++- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 268969fecee7..1e2021bed265 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor) */ static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, uint64_t user_data) + struct drm_crtc *crtc, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; @@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = user_data; + e->event.vbl.crtc_id = crtc->base.id; + e->event.vbl.user_data = user_data; return e; } @@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) { struct drm_pending_vblank_event *e; - e = create_vblank_event(dev, arg->user_data); + e = create_vblank_event(crtc, arg->user_data); if (!e) return -ENOMEM; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 6af02c7b5da3..574fd1475214 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, } e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = page_flip->user_data; + e->event.vbl.user_data = page_flip->user_data; ret = drm_event_reserve_init(dev, file_priv, >base, >event.base); if (ret) { kfree(e); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 47460589d4cd..d250606cc771 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -804,20 +804,23 @@ static void send_vblank_event(struct drm_device *dev, struct drm_pending_vblank_event *e, u64 seq, ktime_t now) { - struct timespec64 tv = ktime_to_timespec64(now); - - e->event.sequence = seq; - /* -* e->event is a user space structure, with hardcoded unsigned -* 32-bit seconds/microseconds. This is safe as we always use -* monotonic timestamps since linux-4.15 -*/ - e->event.tv_sec = tv.tv_sec; - e->event.tv_usec = tv.tv_nsec / 1000; - - trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, -e->event.sequence); + struct timespec64 tv; + switch (e->event.base.type) { + case DRM_EVENT_VBLANK: + case DRM_EVENT_FLIP_COMPLETE: + tv = ktime_to_timespec64(now); + e->event.vbl.sequence = seq; + /* +* e->event is a user space structure, with hardcoded unsigned +* 32-bit seconds/microseconds. This is safe as we always use +* monotonic timestamps since linux-4.15 +*/ + e->event.vbl.tv_sec = tv.tv_sec; + e->event.vbl.tv_usec = tv.tv_nsec / 1000; + break; + } + trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); } @@ -869,7 +872,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, e->pipe = pipe; e->
[PATCH 0/3] drm: Add new vblank ioctls [v4]
This series removes portions of the first patch which switched to ktime_t as Arnd Bergmann posted a cleaner patch which did only that. Otherwise, it's the same. -keith
[PATCH 1/3] drm: Widen vblank count to 64-bits [v3]
This modifies the datatypes used by the vblank code to provide 64 bits of vblank count. The driver interfaces have been left using 32 bits of vblank count; all of the code necessary to widen that value for the user API was already included to handle devices returning fewer than 32-bits. This will provide the necessary datatypes for the Vulkan API. v2: * Re-write wait_vblank ioctl to ABSOLUTE sequence When an application uses the WAIT_VBLANK ioctl with RELATIVE or NEXTONMISS bits set, the target vblank interval is updated within the kernel. We need to write that target back to the ioctl buffer and update the flags bits so that if the wait is interrupted by a signal, when it is re-started, it will target precisely the same vblank count as before. * Leave driver API with 32-bit vblank count v3: * Rebase on top of Arnd Bergmann's patch which had the switch to ktime_t parts. Suggested-by: Michel Dänzer <mic...@daenzer.net> Suggested-by: Daniel Vetter <dan...@ffwll.ch> Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_vblank.c | 104 +-- include/drm/drm_vblank.h | 10 +++-- 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 810a93fc558b..47460589d4cd 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -251,7 +251,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, } DRM_DEBUG_VBL("updating vblank count on crtc %u:" - " current=%u, diff=%u, hw=%u hw_last=%u\n", + " current=%llu, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); if (diff == 0) { @@ -740,17 +740,31 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, * Returns: * The software vblank counter. */ -u32 drm_crtc_vblank_count(struct drm_crtc *crtc) +u64 drm_crtc_vblank_count(struct drm_crtc *crtc) { return drm_vblank_count(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_vblank_count); -static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, +/** + * drm_vblank_count_and_time - retrieve "cooked" vblank counter value and the + * system timestamp corresponding to that vblank counter value. + * @dev: DRM device + * @pipe: index of CRTC whose counter to retrieve + * @vblanktime: Pointer to ktime_t to receive the vblank timestamp. + * + * Fetches the "cooked" vblank count value that represents the number of + * vblank events since the system was booted, including lost events due to + * modesetting activity. Returns corresponding system timestamp of the time + * of the vblank interval that corresponds to the current vblank counter value. + * + * This is the legacy version of drm_crtc_vblank_count_and_time(). + */ +static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, ktime_t *vblanktime) { struct drm_vblank_crtc *vblank = >vblank[pipe]; - u32 vblank_count; + u64 vblank_count; unsigned int seq; if (WARN_ON(pipe >= dev->num_crtcs)) { @@ -778,7 +792,7 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, * modesetting activity. Returns corresponding system timestamp of the time * of the vblank interval that corresponds to the current vblank counter value. */ -u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, +u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, ktime_t *vblanktime) { return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc), @@ -788,7 +802,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); static void send_vblank_event(struct drm_device *dev, struct drm_pending_vblank_event *e, - unsigned long seq, ktime_t now) + u64 seq, ktime_t now) { struct timespec64 tv = ktime_to_timespec64(now); @@ -854,7 +868,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, assert_spin_locked(>event_lock); e->pipe = pipe; - e->event.sequence = drm_vblank_count(dev, pipe); + e->sequence = drm_vblank_count(dev, pipe); e->event.crtc_id = crtc->base.id; list_add_tail(>base.link, >vblank_event_list); } @@ -875,7 +889,8 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, struct drm_pending_vblank_event *e) { struct drm_device *dev = crtc->dev; - unsigned int seq, pipe = drm_crtc_index(crtc); + u64 seq; + unsigned int pipe = drm_crtc_index(crtc); ktime_t now; if (dev->num_crtcs > 0) { @@ -1088,7 +1103,7 @@ void
[PATCH 0/3] drm: Add new vblank ioctls [v4]
This series removes portions of the first patch which switched to ktime_t as Arnd Bergmann posted a cleaner patch which did only that. Otherwise, it's the same. -keith
[PATCH 1/3] drm: Widen vblank count to 64-bits [v3]
This modifies the datatypes used by the vblank code to provide 64 bits of vblank count. The driver interfaces have been left using 32 bits of vblank count; all of the code necessary to widen that value for the user API was already included to handle devices returning fewer than 32-bits. This will provide the necessary datatypes for the Vulkan API. v2: * Re-write wait_vblank ioctl to ABSOLUTE sequence When an application uses the WAIT_VBLANK ioctl with RELATIVE or NEXTONMISS bits set, the target vblank interval is updated within the kernel. We need to write that target back to the ioctl buffer and update the flags bits so that if the wait is interrupted by a signal, when it is re-started, it will target precisely the same vblank count as before. * Leave driver API with 32-bit vblank count v3: * Rebase on top of Arnd Bergmann's patch which had the switch to ktime_t parts. Suggested-by: Michel Dänzer Suggested-by: Daniel Vetter Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_vblank.c | 104 +-- include/drm/drm_vblank.h | 10 +++-- 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 810a93fc558b..47460589d4cd 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -251,7 +251,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, } DRM_DEBUG_VBL("updating vblank count on crtc %u:" - " current=%u, diff=%u, hw=%u hw_last=%u\n", + " current=%llu, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); if (diff == 0) { @@ -740,17 +740,31 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, * Returns: * The software vblank counter. */ -u32 drm_crtc_vblank_count(struct drm_crtc *crtc) +u64 drm_crtc_vblank_count(struct drm_crtc *crtc) { return drm_vblank_count(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_vblank_count); -static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, +/** + * drm_vblank_count_and_time - retrieve "cooked" vblank counter value and the + * system timestamp corresponding to that vblank counter value. + * @dev: DRM device + * @pipe: index of CRTC whose counter to retrieve + * @vblanktime: Pointer to ktime_t to receive the vblank timestamp. + * + * Fetches the "cooked" vblank count value that represents the number of + * vblank events since the system was booted, including lost events due to + * modesetting activity. Returns corresponding system timestamp of the time + * of the vblank interval that corresponds to the current vblank counter value. + * + * This is the legacy version of drm_crtc_vblank_count_and_time(). + */ +static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, ktime_t *vblanktime) { struct drm_vblank_crtc *vblank = >vblank[pipe]; - u32 vblank_count; + u64 vblank_count; unsigned int seq; if (WARN_ON(pipe >= dev->num_crtcs)) { @@ -778,7 +792,7 @@ static u32 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, * modesetting activity. Returns corresponding system timestamp of the time * of the vblank interval that corresponds to the current vblank counter value. */ -u32 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, +u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, ktime_t *vblanktime) { return drm_vblank_count_and_time(crtc->dev, drm_crtc_index(crtc), @@ -788,7 +802,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); static void send_vblank_event(struct drm_device *dev, struct drm_pending_vblank_event *e, - unsigned long seq, ktime_t now) + u64 seq, ktime_t now) { struct timespec64 tv = ktime_to_timespec64(now); @@ -854,7 +868,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, assert_spin_locked(>event_lock); e->pipe = pipe; - e->event.sequence = drm_vblank_count(dev, pipe); + e->sequence = drm_vblank_count(dev, pipe); e->event.crtc_id = crtc->base.id; list_add_tail(>base.link, >vblank_event_list); } @@ -875,7 +889,8 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, struct drm_pending_vblank_event *e) { struct drm_device *dev = crtc->dev; - unsigned int seq, pipe = drm_crtc_index(crtc); + u64 seq; + unsigned int pipe = drm_crtc_index(crtc); ktime_t now; if (dev->num_crtcs > 0) { @@ -1088,7 +1103,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) ktime_t now; unsigne
[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
These provide crtc-id based functions instead of pipe-number, while also offering higher resolution time (ns) and wider frame count (64) as required by the Vulkan API. v2: * Check for DRIVER_MODESET in new crtc-based vblank ioctls Failing to check this will oops the driver. * Ensure vblank interupt is running in crtc_get_sequence ioctl The sequence and timing values are not correct while the interrupt is off, so make sure it's running before asking for them. * Short-circuit get_sequence if the counter is enabled and accurate Steal the idea from the code in wait_vblank to avoid the expense of drm_vblank_get/put * Return active state of crtc in crtc_get_sequence ioctl Might be useful for applications that aren't in charge of modesetting? * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls Daniel Vetter prefers these over the old drm_vblank_put/get APIs. * Return s64 ns instead of u64 in new sequence event Suggested-by: Daniel Vetter <dan...@ffwll.ch> Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com> v3: * Removed FIRST_PIXEL_OUT_FLAG * Document that the timestamp in the query and event are that of the first pixel leaving the display engine for the display (using the same wording as the Vulkan spec). Suggested-by: Michel Dänzer <mic...@daenzer.net> Acked-by: Dave Airlie <airl...@redhat.com> Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_internal.h | 6 ++ drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_vblank.c | 168 + include/drm/drm_vblank.h | 1 + include/uapi/drm/drm.h | 36 + 5 files changed, 213 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index edd921adcf33..c9d5a6cd4d41 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -70,6 +70,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data, int drm_legacy_irq_control(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + +int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a78f03155466..9c435a4c0c82 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index d250606cc771..deb080f62a29 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -819,6 +819,11 @@ static void send_vblank_event(struct drm_device *dev, e->event.vbl.tv_sec = tv.tv_sec; e->event.vbl.tv_usec = tv.tv_nsec / 1000; break; + case DRM_EVENT_CRTC_SEQUENCE: + if (seq) + e->event.seq.sequence = seq; + e->event.seq.time_ns = ktime_to_ns(now); + break; } trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); @@ -1650,3 +1655,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc) return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_handle_vblank); + +/* + * Get crtc VBLANK count. + * + * \param dev DRM device + * \param data user arguement, pointing to a drm_crtc_get_sequence structure. + * \param file_priv drm file private for the user's open file descriptor + */ + +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_crtc *crtc; + struct drm_vblank_crtc *vblank; + int pipe; + struct drm_crtc_get_sequence *get_seq = data; + ktime_t now; + bool vblank_enabled; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + if (!dev->irq_enabled) + return -EINVAL; + + crtc = drm_crtc_find(dev, file_priv, get_seq->
[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
These provide crtc-id based functions instead of pipe-number, while also offering higher resolution time (ns) and wider frame count (64) as required by the Vulkan API. v2: * Check for DRIVER_MODESET in new crtc-based vblank ioctls Failing to check this will oops the driver. * Ensure vblank interupt is running in crtc_get_sequence ioctl The sequence and timing values are not correct while the interrupt is off, so make sure it's running before asking for them. * Short-circuit get_sequence if the counter is enabled and accurate Steal the idea from the code in wait_vblank to avoid the expense of drm_vblank_get/put * Return active state of crtc in crtc_get_sequence ioctl Might be useful for applications that aren't in charge of modesetting? * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls Daniel Vetter prefers these over the old drm_vblank_put/get APIs. * Return s64 ns instead of u64 in new sequence event Suggested-by: Daniel Vetter Suggested-by: Ville Syrjälä v3: * Removed FIRST_PIXEL_OUT_FLAG * Document that the timestamp in the query and event are that of the first pixel leaving the display engine for the display (using the same wording as the Vulkan spec). Suggested-by: Michel Dänzer Acked-by: Dave Airlie Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_internal.h | 6 ++ drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_vblank.c | 168 + include/drm/drm_vblank.h | 1 + include/uapi/drm/drm.h | 36 + 5 files changed, 213 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index edd921adcf33..c9d5a6cd4d41 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -70,6 +70,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data, int drm_legacy_irq_control(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + +int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a78f03155466..9c435a4c0c82 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index d250606cc771..deb080f62a29 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -819,6 +819,11 @@ static void send_vblank_event(struct drm_device *dev, e->event.vbl.tv_sec = tv.tv_sec; e->event.vbl.tv_usec = tv.tv_nsec / 1000; break; + case DRM_EVENT_CRTC_SEQUENCE: + if (seq) + e->event.seq.sequence = seq; + e->event.seq.time_ns = ktime_to_ns(now); + break; } trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); @@ -1650,3 +1655,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc) return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_handle_vblank); + +/* + * Get crtc VBLANK count. + * + * \param dev DRM device + * \param data user arguement, pointing to a drm_crtc_get_sequence structure. + * \param file_priv drm file private for the user's open file descriptor + */ + +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_crtc *crtc; + struct drm_vblank_crtc *vblank; + int pipe; + struct drm_crtc_get_sequence *get_seq = data; + ktime_t now; + bool vblank_enabled; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + if (!dev->irq_enabled) + return -EINVAL; + + crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id); + if (!crtc) + return -ENOENT; + + pipe = drm_crtc_index(crtc); + + vblank = >vblank[pi
Re: [PATCH 2/6] drm: Allow render nodes to query display objects
Daniel Vetterwrites: > One more that came up on irc after discussion why this is needed: The > userspace side using this won't work on split gpus where the render node > has 0 displays, and hence where you really need to query the compositor > anyway. Agreed -- using the X connection to get the information should let this work on split systems. Within the Vulkan code, I already have to move objects from the render node to the display node. Hrm. That exposes a weakness in the kms_display extension I've proposed; that only allows you to pass a single fd. Passing separate display and render fds through that interface seems like a pretty useful change. -- -keith signature.asc Description: PGP signature
Re: [PATCH 2/6] drm: Allow render nodes to query display objects
Daniel Vetter writes: > One more that came up on irc after discussion why this is needed: The > userspace side using this won't work on split gpus where the render node > has 0 displays, and hence where you really need to query the compositor > anyway. Agreed -- using the X connection to get the information should let this work on split systems. Within the Vulkan code, I already have to move objects from the render node to the display node. Hrm. That exposes a weakness in the kms_display extension I've proposed; that only allows you to pass a single fd. Passing separate display and render fds through that interface seems like a pretty useful change. -- -keith signature.asc Description: PGP signature
Re: [PATCH 2/6] drm: Allow render nodes to query display objects
Daniel Vetterwrites: > So given the huge possibilities of abuse, do we really, really need all > this, and is there not any way to create a bit of protocol to pass the > relevant data from X to clients? From your presentation is sounded like > current xrandr is (almost) there ... Yeah, it's the Vulkan EXT_acquire_xlib_display API which is using this access to discover the set of resources available via the render fd in preparation for accessing them over the lease fd once the lease has been created. I *think* I can get enough information to make a good guess as to what kernel resources there are from the RandR info. I'll go give that a try and see if I get stuck. Thanks for the feedback; the fewer kernel API changes the better. -- -keith signature.asc Description: PGP signature
Re: [PATCH 2/6] drm: Allow render nodes to query display objects
Daniel Vetter writes: > So given the huge possibilities of abuse, do we really, really need all > this, and is there not any way to create a bit of protocol to pass the > relevant data from X to clients? From your presentation is sounded like > current xrandr is (almost) there ... Yeah, it's the Vulkan EXT_acquire_xlib_display API which is using this access to discover the set of resources available via the render fd in preparation for accessing them over the lease fd once the lease has been created. I *think* I can get enough information to make a good guess as to what kernel resources there are from the RandR info. I'll go give that a try and see if I get stuck. Thanks for the feedback; the fewer kernel API changes the better. -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Arnd Bergmann <a...@arndb.de> writes: > Overall, this seems good enough, so my patch removes the use of > 'timeval' from the vblank handling altogether and uses ktime_t > consistently, except for the part where we copy the data to user > space structures in the existing format. This patch is better than the portion of my patch which does the same thing as it uses the ktime APIs consistently and doesn't assume that ktime_t is in ns. Thanks much! Reviewed-by: Keith Packard <kei...@keithp.com> -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Arnd Bergmann writes: > Overall, this seems good enough, so my patch removes the use of > 'timeval' from the vblank handling altogether and uses ktime_t > consistently, except for the part where we copy the data to user > space structures in the existing format. This patch is better than the portion of my patch which does the same thing as it uses the ktime APIs consistently and doesn't assume that ktime_t is in ns. Thanks much! Reviewed-by: Keith Packard -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Sean Paulwrites: > Would it be easier for you to respin this into your series, or for me > to just apply it to drm-misc-next? Yeah, I don't see any particular hurry to getting just the time widening patch merged as it doesn't change any external interfaces. I'll go ahead and respin my series using Arnd's time patch first. If Dave decides to delay the vblank series for a long time, we can ask him to just pull the time widening patch in sooner. I won't get to this until tomorrow though. -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Sean Paul writes: > Would it be easier for you to respin this into your series, or for me > to just apply it to drm-misc-next? Yeah, I don't see any particular hurry to getting just the time widening patch merged as it doesn't change any external interfaces. I'll go ahead and respin my series using Arnd's time patch first. If Dave decides to delay the vblank series for a long time, we can ask him to just pull the time widening patch in sooner. I won't get to this until tomorrow though. -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Sean Paulwrites: > It looks like perhaps Keith missed one of the comment tweaks that you have > below. > > Keith, perhaps you can rebase your widening patch on this one? I'm easy; either order works for me just fine. Having the time change separated from the sequence change might be nice? -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Sean Paul writes: > It looks like perhaps Keith missed one of the comment tweaks that you have > below. > > Keith, perhaps you can rebase your widening patch on this one? I'm easy; either order works for me just fine. Having the time change separated from the sequence change might be nice? -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Arnd Bergmannwrites: > That is an interesting coincidence, I created my patch earlier this week > without having any idea that others were looking at the same files. My requirements were to support 64-bit vblank counts and ns precision vblank timing for Vulkan; obviously using ktime_t was a good cleaup at the same time. My patch also widens the sequence numbers to 64-bits, so it's a superset of Arnd's patch; rebasing should be straightforward (although not automatic) if that's what we want to do. -- -keith signature.asc Description: PGP signature
Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval
Arnd Bergmann writes: > That is an interesting coincidence, I created my patch earlier this week > without having any idea that others were looking at the same files. My requirements were to support 64-bit vblank counts and ns precision vblank timing for Vulkan; obviously using ktime_t was a good cleaup at the same time. My patch also widens the sequence numbers to 64-bits, so it's a superset of Arnd's patch; rebasing should be straightforward (although not automatic) if that's what we want to do. -- -keith signature.asc Description: PGP signature
[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]
Place drm_event_vblank in a new union that includes that and a bare drm_event structure. This will allow new members of that union to be added in the future without changing code related to the existing vbl event type. Assignments to the crtc_id field are now done when the event is allocated, rather than when delievered. This way, delivery doesn't need to have the crtc ID available. v2: * Remove 'dev' argument from create_vblank_event It wasn't being used anyways, and if we need it in the future, we can always get it from crtc->dev. * Check for MODESETTING before looking for crtc in queue_vblank_event UMS drivers will oops if we try to get a crtc, so make sure we're modesetting before we try to find a crtc_id to fill into the event. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_atomic.c | 7 --- drivers/gpu/drm/drm_plane.c | 2 +- drivers/gpu/drm/drm_vblank.c | 30 ++ drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- include/drm/drm_vblank.h | 8 +++- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 366c56fe5f58..08ff1b023f7b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor) */ static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, uint64_t user_data) + struct drm_crtc *crtc, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; @@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = user_data; + e->event.vbl.crtc_id = crtc->base.id; + e->event.vbl.user_data = user_data; return e; } @@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) { struct drm_pending_vblank_event *e; - e = create_vblank_event(dev, arg->user_data); + e = create_vblank_event(crtc, arg->user_data); if (!e) return -ENOMEM; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 72cba9805edc..192071209baa 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, } e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = page_flip->user_data; + e->event.vbl.user_data = page_flip->user_data; ret = drm_event_reserve_init(dev, file_priv, >base, >event.base); if (ret) { kfree(e); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 8c3f2fdd821a..7d2674e0362a 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -823,14 +823,16 @@ static void send_vblank_event(struct drm_device *dev, { struct timeval tv; - tv = ktime_to_timeval(now); - e->event.sequence = seq; - e->event.tv_sec = tv.tv_sec; - e->event.tv_usec = tv.tv_usec; - - trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, -e->event.sequence); - + switch (e->event.base.type) { + case DRM_EVENT_VBLANK: + case DRM_EVENT_FLIP_COMPLETE: + tv = ktime_to_timeval(now); + e->event.vbl.sequence = seq; + e->event.vbl.tv_sec = tv.tv_sec; + e->event.vbl.tv_usec = tv.tv_usec; + break; + } + trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); } @@ -882,7 +884,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, e->pipe = pipe; e->sequence = drm_vblank_count(dev, pipe); - e->event.crtc_id = crtc->base.id; list_add_tail(>base.link, >vblank_event_list); } EXPORT_SYMBOL(drm_crtc_arm_vblank_event); @@ -913,7 +914,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, now = get_drm_timestamp(); } e->pipe = pipe; - e->event.crtc_id = crtc->base.id; send_vblank_event(dev, e, seq, now); } EXPORT_SYMBOL(drm_crtc_send_vblank_event); @@ -1347,8 +1347,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int p
[PATCH 2/3] drm: Reorganize drm_pending_event to support future event types [v2]
Place drm_event_vblank in a new union that includes that and a bare drm_event structure. This will allow new members of that union to be added in the future without changing code related to the existing vbl event type. Assignments to the crtc_id field are now done when the event is allocated, rather than when delievered. This way, delivery doesn't need to have the crtc ID available. v2: * Remove 'dev' argument from create_vblank_event It wasn't being used anyways, and if we need it in the future, we can always get it from crtc->dev. * Check for MODESETTING before looking for crtc in queue_vblank_event UMS drivers will oops if we try to get a crtc, so make sure we're modesetting before we try to find a crtc_id to fill into the event. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_atomic.c | 7 --- drivers/gpu/drm/drm_plane.c | 2 +- drivers/gpu/drm/drm_vblank.c | 30 ++ drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- include/drm/drm_vblank.h | 8 +++- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 366c56fe5f58..08ff1b023f7b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1815,7 +1815,7 @@ int drm_atomic_debugfs_init(struct drm_minor *minor) */ static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, uint64_t user_data) + struct drm_crtc *crtc, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; @@ -1825,7 +1825,8 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = user_data; + e->event.vbl.crtc_id = crtc->base.id; + e->event.vbl.user_data = user_data; return e; } @@ -2079,7 +2080,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) { struct drm_pending_vblank_event *e; - e = create_vblank_event(dev, arg->user_data); + e = create_vblank_event(crtc, arg->user_data); if (!e) return -ENOMEM; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 72cba9805edc..192071209baa 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1039,7 +1039,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, } e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); - e->event.user_data = page_flip->user_data; + e->event.vbl.user_data = page_flip->user_data; ret = drm_event_reserve_init(dev, file_priv, >base, >event.base); if (ret) { kfree(e); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 8c3f2fdd821a..7d2674e0362a 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -823,14 +823,16 @@ static void send_vblank_event(struct drm_device *dev, { struct timeval tv; - tv = ktime_to_timeval(now); - e->event.sequence = seq; - e->event.tv_sec = tv.tv_sec; - e->event.tv_usec = tv.tv_usec; - - trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, -e->event.sequence); - + switch (e->event.base.type) { + case DRM_EVENT_VBLANK: + case DRM_EVENT_FLIP_COMPLETE: + tv = ktime_to_timeval(now); + e->event.vbl.sequence = seq; + e->event.vbl.tv_sec = tv.tv_sec; + e->event.vbl.tv_usec = tv.tv_usec; + break; + } + trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); } @@ -882,7 +884,6 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, e->pipe = pipe; e->sequence = drm_vblank_count(dev, pipe); - e->event.crtc_id = crtc->base.id; list_add_tail(>base.link, >vblank_event_list); } EXPORT_SYMBOL(drm_crtc_arm_vblank_event); @@ -913,7 +914,6 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, now = get_drm_timestamp(); } e->pipe = pipe; - e->event.crtc_id = crtc->base.id; send_vblank_event(dev, e, seq, now); } EXPORT_SYMBOL(drm_crtc_send_vblank_event); @@ -1347,8 +1347,14 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe, e->pipe = pipe;
[PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]
This modifies the datatypes used by the vblank code to provide both 64 bits of vblank count and switch to using ktime_t for timestamps to increase resolution from microseconds to nanoseconds. The driver interfaces have been left using 32 bits of vblank count; all of the code necessary to widen that value for the user API was already included to handle devices returning fewer than 32-bits. This will provide the necessary datatypes for the Vulkan API. v2: * Re-write wait_vblank ioctl to ABSOLUTE sequence When an application uses the WAIT_VBLANK ioctl with RELATIVE or NEXTONMISS bits set, the target vblank interval is updated within the kernel. We need to write that target back to the ioctl buffer and update the flags bits so that if the wait is interrupted by a signal, when it is re-started, it will target precisely the same vblank count as before. * Leave driver API with 32-bit vblank count Suggested-by: Michel Dänzer <mic...@daenzer.net> Suggested-by: Daniel Vetter <dan...@ffwll.ch> Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_vblank.c | 214 +++ include/drm/drm_drv.h| 2 +- include/drm/drm_vblank.h | 16 ++-- 3 files changed, 147 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 70f2b9593edc..8c3f2fdd821a 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -78,7 +78,7 @@ static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, - struct timeval *tvblank, bool in_vblank_irq); + ktime_t *tvblank, bool in_vblank_irq); static unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ @@ -99,7 +99,7 @@ MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps"); static void store_vblank(struct drm_device *dev, unsigned int pipe, u32 vblank_count_inc, -struct timeval *t_vblank, u32 last) +ktime_t t_vblank, u32 last) { struct drm_vblank_crtc *vblank = >vblank[pipe]; @@ -108,7 +108,7 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, vblank->last = last; write_seqlock(>seqlock); - vblank->time = *t_vblank; + vblank->time = t_vblank; vblank->count += vblank_count_inc; write_sequnlock(>seqlock); } @@ -151,7 +151,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe { u32 cur_vblank; bool rc; - struct timeval t_vblank; + ktime_t t_vblank; int count = DRM_TIMESTAMP_MAXRETRIES; spin_lock(>vblank_time_lock); @@ -171,13 +171,13 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe * interrupt and assign 0 for now, to mark the vblanktimestamp as invalid. */ if (!rc) - t_vblank = (struct timeval) {0, 0}; + t_vblank = 0; /* * +1 to make sure user will never see the same * vblank counter value before and after a modeset */ - store_vblank(dev, pipe, 1, _vblank, cur_vblank); + store_vblank(dev, pipe, 1, t_vblank, cur_vblank); spin_unlock(>vblank_time_lock); } @@ -200,7 +200,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, struct drm_vblank_crtc *vblank = >vblank[pipe]; u32 cur_vblank, diff; bool rc; - struct timeval t_vblank; + ktime_t t_vblank; int count = DRM_TIMESTAMP_MAXRETRIES; int framedur_ns = vblank->framedur_ns; @@ -225,11 +225,9 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, /* trust the hw counter when it's around */ diff = (cur_vblank - vblank->last) & dev->max_vblank_count; } else if (rc && framedur_ns) { - const struct timeval *t_old; - u64 diff_ns; + ktime_t diff_ns; - t_old = >time; - diff_ns = timeval_to_ns(_vblank) - timeval_to_ns(t_old); + diff_ns = t_vblank - vblank->time; /* * Figure out how many vblanks we've missed based @@ -263,7 +261,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, } DRM_DEBUG_VBL("updating vblank count on crtc %u:" - " current=%u, diff=%u, hw=%u hw_last=%u\n", + " current=%llu, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); if (diff == 0) { @@ -278,9 +276,9 @@ static void drm_update_vblank_count(struct drm_device *dev, uns
[PATCH 1/3] drm: Widen vblank UAPI to 64 bits. Change vblank time to ktime_t [v2]
This modifies the datatypes used by the vblank code to provide both 64 bits of vblank count and switch to using ktime_t for timestamps to increase resolution from microseconds to nanoseconds. The driver interfaces have been left using 32 bits of vblank count; all of the code necessary to widen that value for the user API was already included to handle devices returning fewer than 32-bits. This will provide the necessary datatypes for the Vulkan API. v2: * Re-write wait_vblank ioctl to ABSOLUTE sequence When an application uses the WAIT_VBLANK ioctl with RELATIVE or NEXTONMISS bits set, the target vblank interval is updated within the kernel. We need to write that target back to the ioctl buffer and update the flags bits so that if the wait is interrupted by a signal, when it is re-started, it will target precisely the same vblank count as before. * Leave driver API with 32-bit vblank count Suggested-by: Michel Dänzer Suggested-by: Daniel Vetter Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_vblank.c | 214 +++ include/drm/drm_drv.h| 2 +- include/drm/drm_vblank.h | 16 ++-- 3 files changed, 147 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 70f2b9593edc..8c3f2fdd821a 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -78,7 +78,7 @@ static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, - struct timeval *tvblank, bool in_vblank_irq); + ktime_t *tvblank, bool in_vblank_irq); static unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ @@ -99,7 +99,7 @@ MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps"); static void store_vblank(struct drm_device *dev, unsigned int pipe, u32 vblank_count_inc, -struct timeval *t_vblank, u32 last) +ktime_t t_vblank, u32 last) { struct drm_vblank_crtc *vblank = >vblank[pipe]; @@ -108,7 +108,7 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, vblank->last = last; write_seqlock(>seqlock); - vblank->time = *t_vblank; + vblank->time = t_vblank; vblank->count += vblank_count_inc; write_sequnlock(>seqlock); } @@ -151,7 +151,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe { u32 cur_vblank; bool rc; - struct timeval t_vblank; + ktime_t t_vblank; int count = DRM_TIMESTAMP_MAXRETRIES; spin_lock(>vblank_time_lock); @@ -171,13 +171,13 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe * interrupt and assign 0 for now, to mark the vblanktimestamp as invalid. */ if (!rc) - t_vblank = (struct timeval) {0, 0}; + t_vblank = 0; /* * +1 to make sure user will never see the same * vblank counter value before and after a modeset */ - store_vblank(dev, pipe, 1, _vblank, cur_vblank); + store_vblank(dev, pipe, 1, t_vblank, cur_vblank); spin_unlock(>vblank_time_lock); } @@ -200,7 +200,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, struct drm_vblank_crtc *vblank = >vblank[pipe]; u32 cur_vblank, diff; bool rc; - struct timeval t_vblank; + ktime_t t_vblank; int count = DRM_TIMESTAMP_MAXRETRIES; int framedur_ns = vblank->framedur_ns; @@ -225,11 +225,9 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, /* trust the hw counter when it's around */ diff = (cur_vblank - vblank->last) & dev->max_vblank_count; } else if (rc && framedur_ns) { - const struct timeval *t_old; - u64 diff_ns; + ktime_t diff_ns; - t_old = >time; - diff_ns = timeval_to_ns(_vblank) - timeval_to_ns(t_old); + diff_ns = t_vblank - vblank->time; /* * Figure out how many vblanks we've missed based @@ -263,7 +261,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, } DRM_DEBUG_VBL("updating vblank count on crtc %u:" - " current=%u, diff=%u, hw=%u hw_last=%u\n", + " current=%llu, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); if (diff == 0) { @@ -278,9 +276,9 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, * for now, to mark the vblanktimestamp as in
[PATCH 4/6] drm: Add drm_object lease infrastructure [v4]
This provides new data structures to hold "lease" information about drm mode setting objects, and provides for creating new drm_masters which have access to a subset of the available drm resources. An 'owner' is a drm_master which is not leasing the objects from another drm_master, and hence 'owns' them. A 'lessee' is a drm_master which is leasing objects from some other drm_master. Each lessee holds the set of objects which it is leasing from the lessor. A 'lessor' is a drm_master which is leasing objects to another drm_master. This is the same as the owner in the current code. The set of objects any drm_master 'controls' is limited to the set of objects it leases (for lessees) or all objects (for owners). Objects not controlled by a drm_master cannot be modified through the various state manipulating ioctls, and any state reported back to user space will be edited to make them appear idle and/or unusable. For instance, connectors always report 'disconnected', while encoders report no possible crtcs or clones. The full list of lessees leasing objects from an owner (either directly, or indirectly through another lessee), can be searched from an idr in the drm_master of the owner. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: * Sub-leasing has been disabled. * BUG_ON for lock checking replaced with lockdep_assert_held * 'change' ioctl has been removed. * Leased objects can always be controlled by the lessor; the 'mask_lease' flag has been removed * Checking for leased status has been simplified, replacing the drm_lease_check function with drm_lease_held. Changes in v3, some suggested by Dave Airlie <airl...@gmail.com> * Add revocation. This allows leases to be effectively revoked by removing all of the objects they have access to. The lease itself hangs around as it's hanging off a file. * Free the leases IDR when the master is destroyed * _drm_lease_held should look at lessees, not lessor * Allow non-master files to check for lease status Changes in v4, suggested by Dave Airlie <airl...@gmail.com> * Formatting and whitespace changes Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_auth.c| 29 +++- drivers/gpu/drm/drm_lease.c | 379 ++ include/drm/drm_auth.h| 20 +++ include/drm/drm_lease.h | 36 include/drm/drm_mode_object.h | 1 + 6 files changed, 465 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_lease.c create mode 100644 include/drm/drm_lease.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a3fdc5a68dff..81ff79336623 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -17,7 +17,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ - drm_syncobj.o + drm_syncobj.o drm_lease.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 7ff697389d74..541177c71d51 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -31,6 +31,7 @@ #include #include "drm_internal.h" #include "drm_legacy.h" +#include /** * DOC: master and authentication @@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data, return file ? 0 : -EINVAL; } -static struct drm_master *drm_master_create(struct drm_device *dev) +struct drm_master *drm_master_create(struct drm_device *dev) { struct drm_master *master; @@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev) idr_init(>magic_map); master->dev = dev; + /* initialize the tree of output resource lessees */ + master->lessor = NULL; + master->lessee_id = 0; + INIT_LIST_HEAD(>lessees); + INIT_LIST_HEAD(>lessee_list); + idr_init(>leases); + idr_init(>lessee_idr); + return master; } @@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; } + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(>master_mutex); @@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv) if (dev->master == file_priv->master) drm_drop_master(dev, file_priv); ou
[PATCH 4/6] drm: Add drm_object lease infrastructure [v4]
This provides new data structures to hold "lease" information about drm mode setting objects, and provides for creating new drm_masters which have access to a subset of the available drm resources. An 'owner' is a drm_master which is not leasing the objects from another drm_master, and hence 'owns' them. A 'lessee' is a drm_master which is leasing objects from some other drm_master. Each lessee holds the set of objects which it is leasing from the lessor. A 'lessor' is a drm_master which is leasing objects to another drm_master. This is the same as the owner in the current code. The set of objects any drm_master 'controls' is limited to the set of objects it leases (for lessees) or all objects (for owners). Objects not controlled by a drm_master cannot be modified through the various state manipulating ioctls, and any state reported back to user space will be edited to make them appear idle and/or unusable. For instance, connectors always report 'disconnected', while encoders report no possible crtcs or clones. The full list of lessees leasing objects from an owner (either directly, or indirectly through another lessee), can be searched from an idr in the drm_master of the owner. Changes for v2 as suggested by Daniel Vetter : * Sub-leasing has been disabled. * BUG_ON for lock checking replaced with lockdep_assert_held * 'change' ioctl has been removed. * Leased objects can always be controlled by the lessor; the 'mask_lease' flag has been removed * Checking for leased status has been simplified, replacing the drm_lease_check function with drm_lease_held. Changes in v3, some suggested by Dave Airlie * Add revocation. This allows leases to be effectively revoked by removing all of the objects they have access to. The lease itself hangs around as it's hanging off a file. * Free the leases IDR when the master is destroyed * _drm_lease_held should look at lessees, not lessor * Allow non-master files to check for lease status Changes in v4, suggested by Dave Airlie * Formatting and whitespace changes Signed-off-by: Keith Packard --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_auth.c| 29 +++- drivers/gpu/drm/drm_lease.c | 379 ++ include/drm/drm_auth.h| 20 +++ include/drm/drm_lease.h | 36 include/drm/drm_mode_object.h | 1 + 6 files changed, 465 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_lease.c create mode 100644 include/drm/drm_lease.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index a3fdc5a68dff..81ff79336623 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -17,7 +17,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ - drm_syncobj.o + drm_syncobj.o drm_lease.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 7ff697389d74..541177c71d51 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -31,6 +31,7 @@ #include #include "drm_internal.h" #include "drm_legacy.h" +#include /** * DOC: master and authentication @@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data, return file ? 0 : -EINVAL; } -static struct drm_master *drm_master_create(struct drm_device *dev) +struct drm_master *drm_master_create(struct drm_device *dev) { struct drm_master *master; @@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev) idr_init(>magic_map); master->dev = dev; + /* initialize the tree of output resource lessees */ + master->lessor = NULL; + master->lessee_id = 0; + INIT_LIST_HEAD(>lessees); + INIT_LIST_HEAD(>lessee_list); + idr_init(>leases); + idr_init(>lessee_idr); + return master; } @@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; } + if (file_priv->master->lessor != NULL) { + DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id); + ret = -EINVAL; + goto out_unlock; + } + ret = drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(>master_mutex); @@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv) if (dev->master == file_priv->master) drm_drop_master(dev, file_priv); out: + if (file_priv->is_master) { + /* Revoke any leases held by this or lessees, but only if +
[PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v5]
drm_mode_create_lease Creates a lease for a list of drm mode objects, returning an fd for the new drm_master and a 64-bit identifier for the lessee drm_mode_list_lesees List the identifiers of the lessees for a master file drm_mode_get_lease List the leased objects for a master file drm_mode_revoke_lease Erase the set of objects managed by a lease. This should suffice to at least create and query leases. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: * query ioctls only query the master associated with the provided file. * 'mask_lease' value has been removed * change ioctl has been removed. Changes for v3 suggested in part by Dave Airlie <airl...@gmail.com> * Add revoke ioctl. Changes for v4 suggested by Dave Airlie <airl...@gmail.com> * Expand on the comment about the magic use of _lease_idr_object * Pad lease ioctl structures to align on 64-bit boundaries Changes for v5 suggested by Dave Airlie <airl...@gmail.com> * Check for non-negative object_id in create_lease to avoid debug output from the kernel. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_ioctl.c | 4 + drivers/gpu/drm/drm_lease.c | 282 +++ drivers/gpu/drm/drm_vblank.c | 4 +- include/drm/drm_lease.h | 12 ++ include/uapi/drm/drm.h | 5 + include/uapi/drm/drm_mode.h | 66 ++ 6 files changed, 371 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 78a0d6996e12..c37dc4478d83 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 2ac404264d75..69b54cb557ce 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -23,6 +23,8 @@ #define drm_for_each_lessee(lessee, lessor) \ list_for_each_entry((lessee), &(lessor)->lessees, lessee_list) +static uint64_t drm_lease_idr_object; + /** * drm_lease_owner - return ancestor owner drm_master * @master: drm_master somewhere within tree of lessees and lessors @@ -377,3 +379,283 @@ void _drm_lease_revoke(struct drm_master *top) } } } + +/** + * drm_mode_create_lease_ioctl - create a new lease + * @dev: the drm device + * @data: pointer to struct drm_mode_create_lease + * @file_priv: the file being manipulated + * + * The master associated with the specified file will have a lease + * created containing the objects specified in the ioctl structure. + * A file descriptor will be allocated for that and returned to the + * application. + */ +int drm_mode_create_lease_ioctl(struct drm_device *dev, + void *data, struct drm_file *lessor_priv) +{ + struct drm_mode_create_lease *cl = data; + size_t object_count; + size_t o; + int ret = 0; + struct idr leases; + struct drm_master *lessor = lessor_priv->master; + struct drm_master *lessee = NULL; + struct file *lessee_file = NULL; + struct file *lessor_file = lessor_priv->filp; + struct drm_file *lessee_priv; + int fd = -1; + + /* Do not allow sub-leases */ + if (lessor->lessor) + return -EINVAL; + + object_count = cl->object_count; + idr_init(); + + /* Allocate a file descriptor for the lease */ + fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK)); + + DRM_DEBUG_LEASE("Creating new lease\n"); + + /* Lookup the mode objects and add their IDs to the lease request */ + for (o = 0; o < object_count; o++) { + __u32 object_id; + + if (copy_from_user(_id, + u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32), + sizeof (__u32))) { + ret = -EFAULT; + goto out_leases; + } + DRM_DEBUG_LEASE("Adding
[PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v5]
drm_mode_create_lease Creates a lease for a list of drm mode objects, returning an fd for the new drm_master and a 64-bit identifier for the lessee drm_mode_list_lesees List the identifiers of the lessees for a master file drm_mode_get_lease List the leased objects for a master file drm_mode_revoke_lease Erase the set of objects managed by a lease. This should suffice to at least create and query leases. Changes for v2 as suggested by Daniel Vetter : * query ioctls only query the master associated with the provided file. * 'mask_lease' value has been removed * change ioctl has been removed. Changes for v3 suggested in part by Dave Airlie * Add revoke ioctl. Changes for v4 suggested by Dave Airlie * Expand on the comment about the magic use of _lease_idr_object * Pad lease ioctl structures to align on 64-bit boundaries Changes for v5 suggested by Dave Airlie * Check for non-negative object_id in create_lease to avoid debug output from the kernel. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_ioctl.c | 4 + drivers/gpu/drm/drm_lease.c | 282 +++ drivers/gpu/drm/drm_vblank.c | 4 +- include/drm/drm_lease.h | 12 ++ include/uapi/drm/drm.h | 5 + include/uapi/drm/drm_mode.h | 66 ++ 6 files changed, 371 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 78a0d6996e12..c37dc4478d83 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 2ac404264d75..69b54cb557ce 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -23,6 +23,8 @@ #define drm_for_each_lessee(lessee, lessor) \ list_for_each_entry((lessee), &(lessor)->lessees, lessee_list) +static uint64_t drm_lease_idr_object; + /** * drm_lease_owner - return ancestor owner drm_master * @master: drm_master somewhere within tree of lessees and lessors @@ -377,3 +379,283 @@ void _drm_lease_revoke(struct drm_master *top) } } } + +/** + * drm_mode_create_lease_ioctl - create a new lease + * @dev: the drm device + * @data: pointer to struct drm_mode_create_lease + * @file_priv: the file being manipulated + * + * The master associated with the specified file will have a lease + * created containing the objects specified in the ioctl structure. + * A file descriptor will be allocated for that and returned to the + * application. + */ +int drm_mode_create_lease_ioctl(struct drm_device *dev, + void *data, struct drm_file *lessor_priv) +{ + struct drm_mode_create_lease *cl = data; + size_t object_count; + size_t o; + int ret = 0; + struct idr leases; + struct drm_master *lessor = lessor_priv->master; + struct drm_master *lessee = NULL; + struct file *lessee_file = NULL; + struct file *lessor_file = lessor_priv->filp; + struct drm_file *lessee_priv; + int fd = -1; + + /* Do not allow sub-leases */ + if (lessor->lessor) + return -EINVAL; + + object_count = cl->object_count; + idr_init(); + + /* Allocate a file descriptor for the lease */ + fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK)); + + DRM_DEBUG_LEASE("Creating new lease\n"); + + /* Lookup the mode objects and add their IDs to the lease request */ + for (o = 0; o < object_count; o++) { + __u32 object_id; + + if (copy_from_user(_id, + u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32), + sizeof (__u32))) { + ret = -EFAULT; + goto out_leases; + } + DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id); + + if ((int) object_id < 0) { +
[PATCH 3/6] drm: Add new LEASE debug level
Separate out lease debugging from the core. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_drv.c | 3 ++- include/drm/drmP.h| 4 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c0292e5d7281..a934fd5e7e55 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" -"\t\tBit 5 (0x20) will enable VBL messages (vblank code)"); +"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" +"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)"); module_param_named(debug, drm_debug, int, 0600); static DEFINE_SPINLOCK(drm_minor_lock); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 7277783a4ff0..59be1232d005 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -136,6 +136,7 @@ struct pci_controller; #define DRM_UT_ATOMIC 0x10 #define DRM_UT_VBL 0x20 #define DRM_UT_STATE 0x40 +#define DRM_UT_LEASE 0x80 /***/ /** \name DRM template customization defaults */ @@ -250,6 +251,9 @@ struct pci_controller; #define DRM_DEBUG_VBL(fmt, ...)\ drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__) +#define DRM_DEBUG_LEASE(fmt, ...) \ + drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__) + #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)\ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ -- 2.13.3
[PATCH 3/6] drm: Add new LEASE debug level
Separate out lease debugging from the core. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_drv.c | 3 ++- include/drm/drmP.h| 4 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c0292e5d7281..a934fd5e7e55 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" -"\t\tBit 5 (0x20) will enable VBL messages (vblank code)"); +"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" +"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)"); module_param_named(debug, drm_debug, int, 0600); static DEFINE_SPINLOCK(drm_minor_lock); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 7277783a4ff0..59be1232d005 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -136,6 +136,7 @@ struct pci_controller; #define DRM_UT_ATOMIC 0x10 #define DRM_UT_VBL 0x20 #define DRM_UT_STATE 0x40 +#define DRM_UT_LEASE 0x80 /***/ /** \name DRM template customization defaults */ @@ -250,6 +251,9 @@ struct pci_controller; #define DRM_DEBUG_VBL(fmt, ...)\ drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__) +#define DRM_DEBUG_LEASE(fmt, ...) \ + drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__) + #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)\ ({ \ static DEFINE_RATELIMIT_STATE(_rs, \ -- 2.13.3
[PATCH 5/6] drm: Check mode object lease status in all master ioctl paths [v2]
Attempts to modify un-leased objects are rejected with an error. Information returned about unleased objects is modified to make them appear unusable and/or disconnected. Changes for v2 as suggested by Daniel Vetter <daniel.vet...@ffwll.ch>: With the change in the __drm_mode_object_find API to pass the file_priv along, we can now centralize most of the lease-based access checks in that function. A few places skip that API and require in-line checks. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_auth.c| 2 +- drivers/gpu/drm/drm_connector.c | 5 +++-- drivers/gpu/drm/drm_encoder.c | 8 +--- drivers/gpu/drm/drm_mode_config.c | 32 +++- drivers/gpu/drm/drm_mode_object.c | 22 ++ drivers/gpu/drm/drm_plane.c | 8 +--- drivers/gpu/drm/drm_vblank.c | 18 -- 7 files changed, 71 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 541177c71d51..bab26b477738 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv) */ bool drm_is_current_master(struct drm_file *fpriv) { - return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } EXPORT_SYMBOL(drm_is_current_master); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc8934616..0712c14cb57c 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1315,7 +1315,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, return -ENOENT; for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) - if (connector->encoder_ids[i] != 0) + if (connector->encoder_ids[i] != 0 && + drm_lease_held(file_priv, connector->encoder_ids[i])) encoders_count++; if ((out_resp->count_encoders >= encoders_count) && encoders_count) { @@ -1382,7 +1383,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); encoder = drm_connector_get_encoder(connector); - if (encoder) + if (encoder && drm_lease_held(file_priv, encoder->base.id)) out_resp->encoder_id = encoder->base.id; else out_resp->encoder_id = 0; diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 43f644844b83..6ad6416f2ede 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc) + if (crtc && drm_lease_held(file_priv, crtc->base.id)) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; @@ -234,8 +234,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; - enc_resp->possible_crtcs = encoder->possible_crtcs; - enc_resp->possible_clones = encoder->possible_clones; + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv, + encoder->possible_crtcs); + enc_resp->possible_clones = drm_lease_filter_encoders(file_priv, + encoder->possible_clones); return 0; } diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 74f6ff5df656..fad6646b38d3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -122,20 +122,24 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr); drm_for_each_crtc(crtc, dev) { - if (count < card_res->count_crtcs && - put_user(crtc->base.id, crtc_id + count)) - return -EFAULT; - count++; + if (drm_lease_held(file_priv, crtc->base.id)) { + if (count < card_res->count_crtcs && + put_user(crtc->base.id, crtc_id + count)) + return -EFAULT; + count++; + } } card_res->count_crtcs = count; count = 0; encoder_id = u64_to_user_ptr(card_res->enco
[PATCH 5/6] drm: Check mode object lease status in all master ioctl paths [v2]
Attempts to modify un-leased objects are rejected with an error. Information returned about unleased objects is modified to make them appear unusable and/or disconnected. Changes for v2 as suggested by Daniel Vetter : With the change in the __drm_mode_object_find API to pass the file_priv along, we can now centralize most of the lease-based access checks in that function. A few places skip that API and require in-line checks. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_auth.c| 2 +- drivers/gpu/drm/drm_connector.c | 5 +++-- drivers/gpu/drm/drm_encoder.c | 8 +--- drivers/gpu/drm/drm_mode_config.c | 32 +++- drivers/gpu/drm/drm_mode_object.c | 22 ++ drivers/gpu/drm/drm_plane.c | 8 +--- drivers/gpu/drm/drm_vblank.c | 18 -- 7 files changed, 71 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 541177c71d51..bab26b477738 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -310,7 +310,7 @@ void drm_master_release(struct drm_file *file_priv) */ bool drm_is_current_master(struct drm_file *fpriv) { - return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } EXPORT_SYMBOL(drm_is_current_master); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc8934616..0712c14cb57c 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1315,7 +1315,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, return -ENOENT; for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) - if (connector->encoder_ids[i] != 0) + if (connector->encoder_ids[i] != 0 && + drm_lease_held(file_priv, connector->encoder_ids[i])) encoders_count++; if ((out_resp->count_encoders >= encoders_count) && encoders_count) { @@ -1382,7 +1383,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); encoder = drm_connector_get_encoder(connector); - if (encoder) + if (encoder && drm_lease_held(file_priv, encoder->base.id)) out_resp->encoder_id = encoder->base.id; else out_resp->encoder_id = 0; diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 43f644844b83..6ad6416f2ede 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -226,7 +226,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, drm_modeset_lock(>mode_config.connection_mutex, NULL); crtc = drm_encoder_get_crtc(encoder); - if (crtc) + if (crtc && drm_lease_held(file_priv, crtc->base.id)) enc_resp->crtc_id = crtc->base.id; else enc_resp->crtc_id = 0; @@ -234,8 +234,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data, enc_resp->encoder_type = encoder->encoder_type; enc_resp->encoder_id = encoder->base.id; - enc_resp->possible_crtcs = encoder->possible_crtcs; - enc_resp->possible_clones = encoder->possible_clones; + enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv, + encoder->possible_crtcs); + enc_resp->possible_clones = drm_lease_filter_encoders(file_priv, + encoder->possible_clones); return 0; } diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 74f6ff5df656..fad6646b38d3 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -122,20 +122,24 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr); drm_for_each_crtc(crtc, dev) { - if (count < card_res->count_crtcs && - put_user(crtc->base.id, crtc_id + count)) - return -EFAULT; - count++; + if (drm_lease_held(file_priv, crtc->base.id)) { + if (count < card_res->count_crtcs && + put_user(crtc->base.id, crtc_id + count)) + return -EFAULT; + count++; + } } card_res->count_crtcs = count; count = 0; encoder_id = u64_to_user_ptr(card_res->encoder_id_ptr); drm_for_each_e
[PATCH 0/6] drm: Add drm mode object leases
New since last time: * Fix vboxvideo driver in staging * Fixed some formatting and whitespace errors (Dave Airlie) * Explain the odd use of the idr interface for leases (Dave Airlie) * Disallow negative object_id in lease creation (Dave Airlie) It's also been rebased on top of the drm-sequence-64 sequence just sent out so that merging one and then the other will be simplified.
[PATCH 0/6] drm: Add drm mode object leases
New since last time: * Fix vboxvideo driver in staging * Fixed some formatting and whitespace errors (Dave Airlie) * Explain the odd use of the idr interface for leases (Dave Airlie) * Disallow negative object_id in lease creation (Dave Airlie) It's also been rebased on top of the drm-sequence-64 sequence just sent out so that merging one and then the other will be simplified.
[PATCH 2/6] drm: Allow render nodes to query display objects
This allows an application to discover what display resources are available before requesting a lease from the X server. Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_ioctl.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 25559aa4c65b..78a0d6996e12 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -613,27 +613,27 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATTACHMODE, drm_noop, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DETACHMODE, drm_noop, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPROPERTY, drm_mode_connector_property_set_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), @@ -642,7 +642,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOC
[PATCH 1/6] drm: Pass struct drm_file * to __drm_mode_object_find [v2]
This will allow __drm_mode_object_file to be extended to perform access control checks based on the file in use. v2: Also fix up vboxvideo driver in staging Suggested-by: Daniel Vetter <daniel.vet...@ffwll.ch> Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 ++-- drivers/gpu/drm/ast/ast_mode.c | 2 +- drivers/gpu/drm/bochs/bochs_kms.c| 2 +- drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +- drivers/gpu/drm/drm_atomic.c | 8 drivers/gpu/drm/drm_atomic_helper.c | 2 +- drivers/gpu/drm/drm_color_mgmt.c | 4 ++-- drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_crtc.c | 8 drivers/gpu/drm/drm_crtc_internal.h | 1 + drivers/gpu/drm/drm_encoder.c| 2 +- drivers/gpu/drm/drm_framebuffer.c| 9 + drivers/gpu/drm/drm_mode_object.c| 10 ++ drivers/gpu/drm/drm_plane.c | 14 +++--- drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/drm_property.c | 6 +++--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_overlay.c | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++-- drivers/gpu/drm/radeon/r100.c| 2 +- drivers/gpu/drm/radeon/r600_cs.c | 2 +- drivers/gpu/drm/radeon/radeon_connectors.c | 16 drivers/gpu/drm/udl/udl_connector.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c| 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- drivers/staging/vboxvideo/vbox_mode.c| 2 +- include/drm/drm_connector.h | 3 ++- include/drm/drm_crtc.h | 5 +++-- include/drm/drm_encoder.h| 3 ++- include/drm/drm_framebuffer.h| 1 + include/drm/drm_mode_object.h| 2 ++ include/drm/drm_plane.h | 3 ++- include/drm/drm_property.h | 3 ++- 37 files changed, 85 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index f51b41f094ef..df9cbc78e168 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -231,7 +231,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector, if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -256,7 +256,7 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -372,7 +372,7 @@ amdgpu_connector_best_single_encoder(struct drm_connector *connector) /* pick the encoder ids */ if (enc_id) - return drm_encoder_find(connector->dev, enc_id); + return drm_encoder_find(connector->dev, NULL, enc_id); return NULL; } @@ -1077,7 +1077,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]); + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -1134,7 +1134,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector) if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]); + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue
[PATCH 2/6] drm: Allow render nodes to query display objects
This allows an application to discover what display resources are available before requesting a lease from the X server. Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_ioctl.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 25559aa4c65b..78a0d6996e12 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -613,27 +613,27 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETENCODER, drm_mode_getencoder, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCONNECTOR, drm_mode_getconnector, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATTACHMODE, drm_noop, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DETACHMODE, drm_noop, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPERTY, drm_mode_getproperty_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPROPERTY, drm_mode_connector_property_set_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), @@ -642,7 +642,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2
[PATCH 1/6] drm: Pass struct drm_file * to __drm_mode_object_find [v2]
This will allow __drm_mode_object_file to be extended to perform access control checks based on the file in use. v2: Also fix up vboxvideo driver in staging Suggested-by: Daniel Vetter Signed-off-by: Keith Packard --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 ++-- drivers/gpu/drm/ast/ast_mode.c | 2 +- drivers/gpu/drm/bochs/bochs_kms.c| 2 +- drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +- drivers/gpu/drm/drm_atomic.c | 8 drivers/gpu/drm/drm_atomic_helper.c | 2 +- drivers/gpu/drm/drm_color_mgmt.c | 4 ++-- drivers/gpu/drm/drm_connector.c | 2 +- drivers/gpu/drm/drm_crtc.c | 8 drivers/gpu/drm/drm_crtc_internal.h | 1 + drivers/gpu/drm/drm_encoder.c| 2 +- drivers/gpu/drm/drm_framebuffer.c| 9 + drivers/gpu/drm/drm_mode_object.c| 10 ++ drivers/gpu/drm/drm_plane.c | 14 +++--- drivers/gpu/drm/drm_probe_helper.c | 2 +- drivers/gpu/drm/drm_property.c | 6 +++--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_overlay.c | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++-- drivers/gpu/drm/radeon/r100.c| 2 +- drivers/gpu/drm/radeon/r600_cs.c | 2 +- drivers/gpu/drm/radeon/radeon_connectors.c | 16 drivers/gpu/drm/udl/udl_connector.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c| 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- drivers/staging/vboxvideo/vbox_mode.c| 2 +- include/drm/drm_connector.h | 3 ++- include/drm/drm_crtc.h | 5 +++-- include/drm/drm_encoder.h| 3 ++- include/drm/drm_framebuffer.h| 1 + include/drm/drm_mode_object.h| 2 ++ include/drm/drm_plane.h | 3 ++- include/drm/drm_property.h | 3 ++- 37 files changed, 85 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index f51b41f094ef..df9cbc78e168 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -231,7 +231,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector, if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -256,7 +256,7 @@ amdgpu_connector_find_encoder(struct drm_connector *connector, for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -372,7 +372,7 @@ amdgpu_connector_best_single_encoder(struct drm_connector *connector) /* pick the encoder ids */ if (enc_id) - return drm_encoder_find(connector->dev, enc_id); + return drm_encoder_find(connector->dev, NULL, enc_id); return NULL; } @@ -1077,7 +1077,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force) if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]); + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -1134,7 +1134,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector) if (connector->encoder_ids[i] == 0) break; - encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]); + encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]); if (!encoder) continue; @@ -1153,7 +1153,7 @@ amdgpu_connector_dvi_enco
[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
These provide crtc-id based functions instead of pipe-number, while also offering higher resolution time (ns) and wider frame count (64) as required by the Vulkan API. v2: * Check for DRIVER_MODESET in new crtc-based vblank ioctls Failing to check this will oops the driver. * Ensure vblank interupt is running in crtc_get_sequence ioctl The sequence and timing values are not correct while the interrupt is off, so make sure it's running before asking for them. * Short-circuit get_sequence if the counter is enabled and accurate Steal the idea from the code in wait_vblank to avoid the expense of drm_vblank_get/put * Return active state of crtc in crtc_get_sequence ioctl Might be useful for applications that aren't in charge of modesetting? * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls Daniel Vetter prefers these over the old drm_vblank_put/get APIs. * Return s64 ns instead of u64 in new sequence event Suggested-by: Daniel Vetter <dan...@ffwll.ch> Suggested-by: Ville Syrjälä <ville.syrj...@linux.intel.com> v3: * Removed FIRST_PIXEL_OUT_FLAG * Document that the timestamp in the query and event are that of the first pixel leaving the display engine for the display (using the same wording as the Vulkan spec). Suggested-by: Michel Dänzer <mic...@daenzer.net> Acked-by: Dave Airlie <airl...@redhat.com> Signed-off-by: Keith Packard <kei...@keithp.com> --- drivers/gpu/drm/drm_internal.h | 6 ++ drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_vblank.c | 168 + include/drm/drm_vblank.h | 1 + include/uapi/drm/drm.h | 36 + 5 files changed, 213 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index fbc3f308fa19..ba3e12eeb63d 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -71,6 +71,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data, int drm_legacy_irq_control(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + +int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a9ae6dd2d593..25559aa4c65b 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 7d2674e0362a..45ac2efd5078 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -831,6 +831,11 @@ static void send_vblank_event(struct drm_device *dev, e->event.vbl.tv_sec = tv.tv_sec; e->event.vbl.tv_usec = tv.tv_usec; break; + case DRM_EVENT_CRTC_SEQUENCE: + if (seq) + e->event.seq.sequence = seq; + e->event.seq.time_ns = ktime_to_ns(now); + break; } trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); @@ -1675,3 +1680,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc) return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_handle_vblank); + +/* + * Get crtc VBLANK count. + * + * \param dev DRM device + * \param data user arguement, pointing to a drm_crtc_get_sequence structure. + * \param file_priv drm file private for the user's open file descriptor + */ + +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_crtc *crtc; + struct drm_vblank_crtc *vblank; + int pipe; + struct drm_crtc_get_sequence *get_seq = data; + ktime_t now; + bool vblank_enabled; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + if (!dev->irq_enabled) + return -EINVAL; + + crtc = drm_crtc_find(dev, get_seq->crtc_id); +
[PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]
These provide crtc-id based functions instead of pipe-number, while also offering higher resolution time (ns) and wider frame count (64) as required by the Vulkan API. v2: * Check for DRIVER_MODESET in new crtc-based vblank ioctls Failing to check this will oops the driver. * Ensure vblank interupt is running in crtc_get_sequence ioctl The sequence and timing values are not correct while the interrupt is off, so make sure it's running before asking for them. * Short-circuit get_sequence if the counter is enabled and accurate Steal the idea from the code in wait_vblank to avoid the expense of drm_vblank_get/put * Return active state of crtc in crtc_get_sequence ioctl Might be useful for applications that aren't in charge of modesetting? * Use drm_crtc_vblank_get/put in new crtc-based vblank sequence ioctls Daniel Vetter prefers these over the old drm_vblank_put/get APIs. * Return s64 ns instead of u64 in new sequence event Suggested-by: Daniel Vetter Suggested-by: Ville Syrjälä v3: * Removed FIRST_PIXEL_OUT_FLAG * Document that the timestamp in the query and event are that of the first pixel leaving the display engine for the display (using the same wording as the Vulkan spec). Suggested-by: Michel Dänzer Acked-by: Dave Airlie Signed-off-by: Keith Packard --- drivers/gpu/drm/drm_internal.h | 6 ++ drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_vblank.c | 168 + include/drm/drm_vblank.h | 1 + include/uapi/drm/drm.h | 36 + 5 files changed, 213 insertions(+) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index fbc3f308fa19..ba3e12eeb63d 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -71,6 +71,12 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data, int drm_legacy_irq_control(struct drm_device *dev, void *data, struct drm_file *file_priv); +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + +int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); + /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a9ae6dd2d593..25559aa4c65b 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -663,6 +663,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 7d2674e0362a..45ac2efd5078 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -831,6 +831,11 @@ static void send_vblank_event(struct drm_device *dev, e->event.vbl.tv_sec = tv.tv_sec; e->event.vbl.tv_usec = tv.tv_usec; break; + case DRM_EVENT_CRTC_SEQUENCE: + if (seq) + e->event.seq.sequence = seq; + e->event.seq.time_ns = ktime_to_ns(now); + break; } trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); drm_send_event_locked(dev, >base); @@ -1675,3 +1680,166 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc) return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_handle_vblank); + +/* + * Get crtc VBLANK count. + * + * \param dev DRM device + * \param data user arguement, pointing to a drm_crtc_get_sequence structure. + * \param file_priv drm file private for the user's open file descriptor + */ + +int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_crtc *crtc; + struct drm_vblank_crtc *vblank; + int pipe; + struct drm_crtc_get_sequence *get_seq = data; + ktime_t now; + bool vblank_enabled; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + if (!dev->irq_enabled) + return -EINVAL; + + crtc = drm_crtc_find(dev, get_seq->crtc_id); + if (!crtc) + return -ENOENT; + + pipe = drm_crtc_index(crtc); + + vblank = >vblank[pipe]; + vblank_enable
[PATCH 0/3] drm: add new vblank ioctls [v3]
This version removes the FIRST_PIXEL_OUT flag as the existing code already conforms to that requirement. It has also been rebased to drm-next.
[PATCH 0/3] drm: add new vblank ioctls [v3]
This version removes the FIRST_PIXEL_OUT flag as the existing code already conforms to that requirement. It has also been rebased to drm-next.
Re: [PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v4]
Dave Airliewrites: >> +*/ >> + ret = idr_alloc(, _lease_idr_object , object_id, >> object_id + 1, GFP_KERNEL); > > In current kernels, the idr_alloc warns if start < 0, so if object_id > is a negative signed integer, which your > igt tests actually make it. > > So we get an ugly debug splat from the idr code, you should probably > make sure object_id is > something valid in signed space first, to avoid the splat. Yeah, makes sense. I also found the vboxvideo driver in staging which needed an API tweak. I've pushed that as drm-lease-v5 and will plan on dumping that to the list when you're done with -v4. There are a couple of conflicts and another update needed when merging leases and the new vblank ioctls together; once we've got one of the two ready to go, I can rebase the other on top of that and fix things up. -- -keith signature.asc Description: PGP signature
Re: [PATCH 6/6] drm: Add four ioctls for managing drm mode object leases [v4]
Dave Airlie writes: >> +*/ >> + ret = idr_alloc(, _lease_idr_object , object_id, >> object_id + 1, GFP_KERNEL); > > In current kernels, the idr_alloc warns if start < 0, so if object_id > is a negative signed integer, which your > igt tests actually make it. > > So we get an ugly debug splat from the idr code, you should probably > make sure object_id is > something valid in signed space first, to avoid the splat. Yeah, makes sense. I also found the vboxvideo driver in staging which needed an API tweak. I've pushed that as drm-lease-v5 and will plan on dumping that to the list when you're done with -v4. There are a couple of conflicts and another update needed when merging leases and the new vblank ioctls together; once we've got one of the two ready to go, I can rebase the other on top of that and fix things up. -- -keith signature.asc Description: PGP signature
Re: [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v2]
Daniel Vetterwrites: > I just figured that -modesetting would be the simplest domenstration > vehicle, since the vulkan patches don't look ready yet. I need fully > reviewed userspace before we can land any kernel stuff. Doing > the quick modesetting conversion would unblock. I've provided a patch for the modesetting driver and the preliminary bits are merged, leaving only a fairly straightforward addition of the new ioctls to that code. I'm not sure how to make more progress there at this point; that code would need testing, and it requires a hand-patched kernel to test. I also posted IGT tests for the new functionality, again, getting those reviewed and tested depends on someone willing to build a patched kernel. Dave Airlie has started trying to get that done. >> (regarding FIRST_PIXEL_OUT): >> >> If the timestamp is the only important thing, it sounds like the kernel >> already satisfies that, which is cool. > > Would be good to confirm that. If it's not, we have a problem. Michel Dänzer says that the timestamp provided is computed to be first_pixel_out for all hardware. Given that, I suggest we specify that in the documentation and remove this bit from the API. Seem reasonable? -- -keith signature.asc Description: PGP signature