* [PATCH] drm/lease: allow empty leases
@ 2021-09-02 9:11 Simon Ser
2021-09-02 14:28 ` Pekka Paalanen
0 siblings, 1 reply; 3+ messages in thread
From: Simon Ser @ 2021-09-02 9:11 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, Daniel Stone, Pekka Paalanen, Michel Dänzer,
Emil Velikov, Keith Packard, Boris Brezillon, Dave Airlie
This can be used to create a separate DRM file description, thus
creating a new GEM handle namespace. See [1].
Example usage in wlroots is available at [2].
[1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
[2]: https://github.com/swaywm/wlroots/pull/3158
Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: Michel Dänzer <michel@daenzer.net>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Keith Packard <keithp@keithp.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_lease.c | 39 +++++++++++++++++--------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index dee4f24a1808..d72c2fac0ff1 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -489,12 +489,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- /* need some objects */
- if (cl->object_count == 0) {
- DRM_DEBUG_LEASE("no objects in lease\n");
- return -EINVAL;
- }
-
if (cl->flags && (cl->flags & ~(O_CLOEXEC | O_NONBLOCK))) {
DRM_DEBUG_LEASE("invalid flags\n");
return -EINVAL;
@@ -510,23 +504,26 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
object_count = cl->object_count;
- object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
- array_size(object_count, sizeof(__u32)));
- if (IS_ERR(object_ids)) {
- ret = PTR_ERR(object_ids);
- goto out_lessor;
- }
-
+ /* Handle leased objects, if any */
idr_init(&leases);
+ if (object_count != 0) {
+ object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
+ array_size(object_count, sizeof(__u32)));
+ if (IS_ERR(object_ids)) {
+ ret = PTR_ERR(object_ids);
+ idr_destroy(&leases);
+ goto out_lessor;
+ }
- /* fill and validate the object idr */
- ret = fill_object_idr(dev, lessor_priv, &leases,
- object_count, object_ids);
- kfree(object_ids);
- if (ret) {
- DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
- idr_destroy(&leases);
- goto out_lessor;
+ /* fill and validate the object idr */
+ ret = fill_object_idr(dev, lessor_priv, &leases,
+ object_count, object_ids);
+ kfree(object_ids);
+ if (ret) {
+ DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
+ idr_destroy(&leases);
+ goto out_lessor;
+ }
}
/* Allocate a file descriptor for the lease */
--
2.33.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/lease: allow empty leases
2021-09-02 9:11 [PATCH] drm/lease: allow empty leases Simon Ser
@ 2021-09-02 14:28 ` Pekka Paalanen
2021-09-02 14:47 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Pekka Paalanen @ 2021-09-02 14:28 UTC (permalink / raw)
To: Simon Ser
Cc: dri-devel, Daniel Vetter, Daniel Stone, Michel Dänzer,
Emil Velikov, Keith Packard, Boris Brezillon, Dave Airlie
[-- Attachment #1: Type: text/plain, Size: 3323 bytes --]
On Thu, 02 Sep 2021 09:11:40 +0000
Simon Ser <contact@emersion.fr> wrote:
> This can be used to create a separate DRM file description, thus
> creating a new GEM handle namespace. See [1].
>
> Example usage in wlroots is available at [2].
>
> [1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
> [2]: https://github.com/swaywm/wlroots/pull/3158
>
Hi Simon,
I have a feeling that this is a good idea, but could you explain in
this commit message some real use cases where one needs a new GEM
handle namespace? Not just "when you share a DRM fd between processes"
but *why* you shared a DRM device fd between processes.
If I have trouble remembering or figuring that out from those links,
then I'm sure others have too.
Thanks,
pq
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> Cc: Michel Dänzer <michel@daenzer.net>
> Cc: Emil Velikov <emil.l.velikov@gmail.com>
> Cc: Keith Packard <keithp@keithp.com>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/drm_lease.c | 39 +++++++++++++++++--------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index dee4f24a1808..d72c2fac0ff1 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -489,12 +489,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EOPNOTSUPP;
>
> - /* need some objects */
> - if (cl->object_count == 0) {
> - DRM_DEBUG_LEASE("no objects in lease\n");
> - return -EINVAL;
> - }
> -
> if (cl->flags && (cl->flags & ~(O_CLOEXEC | O_NONBLOCK))) {
> DRM_DEBUG_LEASE("invalid flags\n");
> return -EINVAL;
> @@ -510,23 +504,26 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
>
> object_count = cl->object_count;
>
> - object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
> - array_size(object_count, sizeof(__u32)));
> - if (IS_ERR(object_ids)) {
> - ret = PTR_ERR(object_ids);
> - goto out_lessor;
> - }
> -
> + /* Handle leased objects, if any */
> idr_init(&leases);
> + if (object_count != 0) {
> + object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
> + array_size(object_count, sizeof(__u32)));
> + if (IS_ERR(object_ids)) {
> + ret = PTR_ERR(object_ids);
> + idr_destroy(&leases);
> + goto out_lessor;
> + }
>
> - /* fill and validate the object idr */
> - ret = fill_object_idr(dev, lessor_priv, &leases,
> - object_count, object_ids);
> - kfree(object_ids);
> - if (ret) {
> - DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
> - idr_destroy(&leases);
> - goto out_lessor;
> + /* fill and validate the object idr */
> + ret = fill_object_idr(dev, lessor_priv, &leases,
> + object_count, object_ids);
> + kfree(object_ids);
> + if (ret) {
> + DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
> + idr_destroy(&leases);
> + goto out_lessor;
> + }
> }
>
> /* Allocate a file descriptor for the lease */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/lease: allow empty leases
2021-09-02 14:28 ` Pekka Paalanen
@ 2021-09-02 14:47 ` Daniel Vetter
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2021-09-02 14:47 UTC (permalink / raw)
To: Pekka Paalanen
Cc: Simon Ser, dri-devel, Daniel Vetter, Daniel Stone,
Michel Dänzer, Emil Velikov, Keith Packard, Boris Brezillon,
Dave Airlie
On Thu, Sep 02, 2021 at 05:28:10PM +0300, Pekka Paalanen wrote:
> On Thu, 02 Sep 2021 09:11:40 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
> > This can be used to create a separate DRM file description, thus
> > creating a new GEM handle namespace. See [1].
> >
> > Example usage in wlroots is available at [2].
> >
> > [1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
> > [2]: https://github.com/swaywm/wlroots/pull/3158
> >
>
> Hi Simon,
>
> I have a feeling that this is a good idea, but could you explain in
> this commit message some real use cases where one needs a new GEM
> handle namespace? Not just "when you share a DRM fd between processes"
> but *why* you shared a DRM device fd between processes.
>
> If I have trouble remembering or figuring that out from those links,
> then I'm sure others have too.
Also please document the uapi headers and explain the use-case there and
why and all that.
I'd like that we officiate all uapi we intentionally create in the docs as
much as possible.
Also igt testcase patch for this too pls.
-Daniel
>
>
> Thanks,
> pq
>
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Daniel Stone <daniels@collabora.com>
> > Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > Cc: Emil Velikov <emil.l.velikov@gmail.com>
> > Cc: Keith Packard <keithp@keithp.com>
> > Cc: Boris Brezillon <boris.brezillon@collabora.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > ---
> > drivers/gpu/drm/drm_lease.c | 39 +++++++++++++++++--------------------
> > 1 file changed, 18 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> > index dee4f24a1808..d72c2fac0ff1 100644
> > --- a/drivers/gpu/drm/drm_lease.c
> > +++ b/drivers/gpu/drm/drm_lease.c
> > @@ -489,12 +489,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
> > if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > return -EOPNOTSUPP;
> >
> > - /* need some objects */
> > - if (cl->object_count == 0) {
> > - DRM_DEBUG_LEASE("no objects in lease\n");
> > - return -EINVAL;
> > - }
> > -
> > if (cl->flags && (cl->flags & ~(O_CLOEXEC | O_NONBLOCK))) {
> > DRM_DEBUG_LEASE("invalid flags\n");
> > return -EINVAL;
> > @@ -510,23 +504,26 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
> >
> > object_count = cl->object_count;
> >
> > - object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
> > - array_size(object_count, sizeof(__u32)));
> > - if (IS_ERR(object_ids)) {
> > - ret = PTR_ERR(object_ids);
> > - goto out_lessor;
> > - }
> > -
> > + /* Handle leased objects, if any */
> > idr_init(&leases);
> > + if (object_count != 0) {
> > + object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
> > + array_size(object_count, sizeof(__u32)));
> > + if (IS_ERR(object_ids)) {
> > + ret = PTR_ERR(object_ids);
> > + idr_destroy(&leases);
> > + goto out_lessor;
> > + }
> >
> > - /* fill and validate the object idr */
> > - ret = fill_object_idr(dev, lessor_priv, &leases,
> > - object_count, object_ids);
> > - kfree(object_ids);
> > - if (ret) {
> > - DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
> > - idr_destroy(&leases);
> > - goto out_lessor;
> > + /* fill and validate the object idr */
> > + ret = fill_object_idr(dev, lessor_priv, &leases,
> > + object_count, object_ids);
> > + kfree(object_ids);
> > + if (ret) {
> > + DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
> > + idr_destroy(&leases);
> > + goto out_lessor;
> > + }
> > }
> >
> > /* Allocate a file descriptor for the lease */
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-02 14:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-02 9:11 [PATCH] drm/lease: allow empty leases Simon Ser
2021-09-02 14:28 ` Pekka Paalanen
2021-09-02 14:47 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.