[PATCH] drm/lease: allow empty leases

[Daniel Vetter]

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 at 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 at emersion.fr>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Daniel Stone <daniels at collabora.com>
> > Cc: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > Cc: Michel Dänzer <michel at daenzer.net>
> > Cc: Emil Velikov <emil.l.velikov at gmail.com>
> > Cc: Keith Packard <keithp at keithp.com>
> > Cc: Boris Brezillon <boris.brezillon at collabora.com>
> > Cc: Dave Airlie <airlied at 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