Date: Sun, 2 Apr 2017 15:38:02 +0200
From: Daniel Vetter <daniel@ffwll.ch>
To: Keith Packard <keithp@keithp.com>
Cc: linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com>,
        dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
Message-ID: <20170402133802.wgminf777ksczl76@phenom.ffwll.local>
Mail-Followup-To: Keith Packard <keithp@keithp.com>,
        linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com>,
        dri-devel@lists.freedesktop.org
References: <20170401170841.2643-1-keithp@keithp.com>
 <20170401170841.2643-3-keithp@keithp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20170401170841.2643-3-keithp@keithp.com>
User-Agent: NeoMutt/20170306 (1.8.0)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 5709
Lines: 166

On Sat, Apr 01, 2017 at 10:08:39AM -0700, Keith Packard wrote:
> 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. This sits at the top of a
> tree of drm_masters.
> 
> 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.
> 
> The set of objects any drm_master 'controls' is limited to the set of
> objects it leases (for lessees) or all objects (for owners),
> optionally minus the set of objects it has leased to other
> drm_masters.

Still not sure we want to restrict objects on the lessor side. Feels like
unecessary complexity (i.e. more bugs in kernel, that's never good), and
at best only needed for lessors who can't keep track of stuff. I'm also
not sure whether we really want sub-leases in v1, that's easy to add later
on, but for now just complicates stuff. Main compositor should be a full
master, VR can be the first lease level, we don't need more I think for
now?

> 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.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

[snip]

> diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> index 610223b0481b..e0e2af09d3af 100644
> --- a/include/drm/drm_auth.h
> +++ b/include/drm/drm_auth.h
> @@ -50,10 +50,38 @@ struct drm_master {
>  	struct idr magic_map;
>  	struct drm_lock_data lock;
>  	void *driver_priv;
> +
> +	/* Tree of display resource leases, each of which is a drm_master struct
> +	 * All of these get activated simultaneously, so drm_device master points

&drm_device.master to do a reference in kernel-doc. Please feed this to
kernel-doc in general and make sure the links all point at the right
stuff, and it's all parsed.

Cheers, Daniel


> +	 * at the top of the tree (for which lessor is NULL)
> +	 */
> +
> +	/** Lease holder */

	/** @lessor: Lease holder. */

> +	struct drm_master *lessor;
> +
> +	/** id for lessees. Owners always have id 0 */
> +	int	lessee_id;
> +
> +	/** other lessees of the same master */
> +	struct list_head lessee_list;
> +
> +	/** drm_masters leasing from this one */
> +	struct list_head lessees;
> +
> +	/** Objects leased to this drm_master. */
> +	struct idr leases;
> +
> +	/** All lessees under this owner (only used where lessor == NULL) */
> +	struct idr lessee_idr;
> +
> +	/** Indicates that the leased objects should be hidden from the lessor */
> +	bool mask_lease;
>  };
>  
>  struct drm_master *drm_master_get(struct drm_master *master);
>  void drm_master_put(struct drm_master **master);
>  bool drm_is_current_master(struct drm_file *fpriv);
>  
> +struct drm_master *drm_master_create(struct drm_device *dev);
> +
>  #endif
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> new file mode 100644
> index 000000000000..e02adf3e42fd
> --- /dev/null
> +++ b/include/drm/drm_lease.h
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright ? 2017 Keith Packard <keithp@keithp.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#ifndef _DRM_LEASE_H_
> +#define _DRM_LEASE_H_
> +
> +struct drm_file;
> +struct drm_device;
> +
> +struct drm_master *drm_lease_owner(struct drm_master *master);
> +
> +struct drm_master *drm_lessee_find(struct drm_master *top, int lessee_id);
> +
> +void drm_lease_destroy(struct drm_master *lessee);
> +
> +struct drm_mode_object *drm_lease_find(struct drm_master *master, int id);
> +
> +/**
> + * drm_lease_check - check drm_mode_object lease status
> + * @master: the drm_master
> + * @id: the object id
> + *
> + * Checks if the specified master holds a lease on the object
> + * and also whether it has been leased to some lessee of the
> + * specified master. Return value:
> + *
> + *	0		'master' holds a lease (or owns) and has not leased
> + *	-EACCESS	Some other master holds the lease
> + *	-ENOENT		'id' is not a valid DRM object for this device
> + *	-EBUSY		'master' holds lease, but has sub-leased
> + */
> +
> +static inline int drm_lease_check(struct drm_master *master, int id) {
> +	struct drm_mode_object *object = drm_lease_find(master, id);
> +	if (IS_ERR(object))
> +		return PTR_ERR(object);
> +	return 0;
> +}
> +
> +#endif /* _DRM_LEASE_H_ */
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch