Re: [PATCH] drm/plane: drop num_overlay_planes (v3)

[Daniel Vetter]

On Fri, Oct 13, 2017 at 04:20:02PM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> 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)
> v3: use correct bounds check, found by igt.

Oh sweet, igt is better than I thought \o/

Since this was the only thing I spotted:

Reviewed-by: Daniel Vetter 

> Signed-off-by: Dave Airlie 
> ---
>  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..8d9824804b0c 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(>head, >plane_list);
>   plane->index = config->num_total_plane++;
> - if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> - config->num_overlay_plane++;
>  
>   drm_object_attach_property(>base,
>  config->plane_type_property,
> @@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  
>   list_del(>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 = >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 < plane_resp->count_planes &&
> + 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;
>  
> - /**
> -  * 

[PATCH] drm/plane: drop num_overlay_planes (v3)

[Dave Airlie]

From: Dave Airlie 

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)
v3: use correct bounds check, found by igt.

Signed-off-by: Dave Airlie 
---
 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..8d9824804b0c 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(>head, >plane_list);
plane->index = config->num_total_plane++;
-   if (plane->type == DRM_PLANE_TYPE_OVERLAY)
-   config->num_overlay_plane++;
 
drm_object_attach_property(>base,
   config->plane_type_property,
@@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
 
list_del(>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 = >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 < plane_resp->count_planes &&
+   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