Re: [PATCH 4.19 098/118] drm/lease: Send a distinct uevent

2018-12-11 Thread Keith Packard
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

2018-04-07 Thread Keith Packard
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

2018-04-07 Thread Keith Packard
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

2018-04-06 Thread Keith Packard
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


Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls

2018-04-06 Thread Keith Packard
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

2018-04-06 Thread Keith Packard
(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

2018-04-06 Thread Keith Packard
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

2018-04-06 Thread Keith Packard
(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

2018-04-06 Thread Keith Packard
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()

2018-02-16 Thread Keith Packard
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()

2018-02-16 Thread Keith Packard
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

2018-02-04 Thread Keith Packard
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


Re: [PATCH] hwrng: Explicitly drop current_rng when unregistering rng device

2018-02-04 Thread Keith Packard
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

2018-01-30 Thread Keith Packard
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

2018-01-30 Thread Keith Packard
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

2018-01-30 Thread Keith Packard
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

2018-01-30 Thread Keith Packard
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

2018-01-23 Thread Keith Packard
"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

2018-01-23 Thread Keith Packard
"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

2018-01-18 Thread Keith Packard
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

2018-01-18 Thread Keith Packard
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

2017-12-20 Thread Keith Packard
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

2017-12-20 Thread Keith Packard
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]

2017-12-13 Thread Keith Packard
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]

2017-12-13 Thread Keith Packard
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

2017-12-07 Thread Keith Packard
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

2017-12-07 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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



[PATCH 3/3] drm/edid: quirk HTC vive headset as non-desktop. [v2]

2017-11-10 Thread Keith Packard
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 3/3] drm/edid: quirk HTC vive headset as non-desktop. [v2]

2017-11-10 Thread Keith Packard
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]

2017-11-10 Thread Keith Packard
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]

2017-10-16 Thread Keith Packard
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 5/5] drm: Add four ioctls for managing drm mode object leases [v6]

2017-10-16 Thread Keith Packard
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]

2017-10-16 Thread Keith Packard
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]

2017-10-16 Thread Keith Packard
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

2017-10-16 Thread Keith Packard
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 0/5]: drm: Add drm mode object leases

2017-10-16 Thread Keith Packard
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]

2017-10-15 Thread Keith Packard
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


Re: [PATCH 3/3] drm: Add CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls [v3]

2017-10-15 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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)

2017-10-12 Thread Keith Packard
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, 

[PATCH 1/5] drm/plane: drop num_overlay_planes (v2)

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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]

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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 2/6] drm: Allow render nodes to query display objects

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-12 Thread Keith Packard
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

2017-10-11 Thread Keith Packard
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

2017-10-11 Thread Keith Packard
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

2017-10-11 Thread Keith Packard
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

2017-10-11 Thread Keith Packard
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

2017-10-11 Thread Keith Packard
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


Re: [PATCH 1/2] drm: vblank: use ktime_t instead of timeval

2017-10-11 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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]

2017-10-10 Thread Keith Packard
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 6/6] drm: Add four ioctls for managing drm mode object leases [v4]

2017-10-10 Thread Keith Packard
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]

2017-10-09 Thread Keith Packard
Daniel Vetter  writes:

> 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


  1   2   3   4   >