[PATCH] drm/lease: allow empty leases

[Pekka Paalanen]

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.


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 */

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210902/8bbf49f4/attachment.sig>