Skip to contentSkip to conversation

drm/plane: drop num_overlay_planes

PATCH In review Dave Airlie 2 participants 2 messages Oct 12, 2017 – Oct 16, 2017

Subsystems (2)
lore
Share & tools
lore

Dave Airlie

author maintainer
#0 lore raw

PATCH 1/1 drm/plane: drop num_overlay_planes

From: Dave Airlie <[email protected]>

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.

---
 drivers/gpu/drm/drm_mode_config.c |  1 -
 drivers/gpu/drm/drm_plane.c       | 45 +++++++++++++--------------------------
 include/drm/drm_mode_config.h     | 13 -----------
 3 files changed, 15 insertions(+), 44 deletions(-)
drivers/gpu/drm/drm_mode_config.c +0 -1
index 74f6ff5df656..919e78d45ab0 100644
@@ -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);
drivers/gpu/drm/drm_plane.c +15 -30

74 lines not on this page · raw message

include/drm/drm_mode_config.h +0 -13
index 1b37368416c8..0b4ac2ebc610 100644
@@ -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:
 	 *
Signature · 6 lines
2.14.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · Dave Airlie

From: Dave Airlie <[email protected]>

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.

Pls check for intel CI to double-check this, every time I touched this code I broke something :-/

Quoted 80 lines
---
 drivers/gpu/drm/drm_mode_config.c |  1 -
 drivers/gpu/drm/drm_plane.c       | 45 +++++++++++++--------------------------
 include/drm/drm_mode_config.h     | 13 -----------
 3 files changed, 15 insertions(+), 44 deletions(-)
drivers/gpu/drm/drm_mode_config.c +0 -1
index 74f6ff5df656..919e78d45ab0 100644
@@ -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);
drivers/gpu/drm/drm_plane.c +1 -29
index 6af02c7b5da3..faa34afa708a 100644
@@ -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,32 @@ 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;
-
 	/*
 	 * 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++;
-		}

drivers/gpu/drm/drm_plane.c int drm_mode_getplane_res(struct drm_device *dev, void *data, · line 481

Quoted 10 lines
drivers/gpu/drm/drm_plane.c +10 -0
+	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 &&

Shouldn't this check be count < plane_resp->count_planes? I think we don't have nasty igts yet to check all these, but since we iirc query first with num_planes = 0 and nothing allocated this should blow up.

Otherwise looks like a nice cleanup, with the bug fixed:

Quoted 40 lines
drivers/gpu/drm/drm_plane.c +4 -1
+		    put_user(plane->base.id, plane_ptr + count))
+			return -EFAULT;
+		count++;
 	}
-	plane_resp->count_planes = num_planes;
+	plane_resp->count_planes = count;
 
 	return 0;
 }
include/drm/drm_mode_config.h +0 -13
index 1b37368416c8..0b4ac2ebc610 100644
@@ -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:
 	 *
Signature · 6 lines
2.14.2

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx