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

From: Sean Paul <seanpaul@chromium.org>
To: Keith Packard <keithp@keithp.com>
Cc: linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/5] drm/plane: drop num_overlay_planes (v2)
Date: Mon, 16 Oct 2017 15:23:52 -0400	[thread overview]
Message-ID: <20171016192352.4qqernqwzg4fnxry@art_vandelay> (raw)
In-Reply-To: <20171013015631.6926-2-keithp@keithp.com>

On Thu, Oct 12, 2017 at 06:56:27PM -0700, Keith Packard wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> 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 <airlied@redhat.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  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(&plane->head, &config->plane_list);
>  	plane->index = config->num_total_plane++;
> -	if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> -		config->num_overlay_plane++;
>  
>  	drm_object_attach_property(&plane->base,
>  				   config->plane_type_property,
> @@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  
>  	list_del(&plane->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 = &dev->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 planes. This is invariant over the lifetime of a
> -	 * device and hence doesn't need any locks.
> -	 */
> -	int num_overlay_plane;
>  	/**
>  	 * @num_total_plane:
>  	 *
> -- 
> 2.15.0.rc0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS