diff mbox

drm: Store the plane's index

Message ID 1464255297-13015-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 26, 2016, 9:34 a.m. UTC
Currently the plane's index is determined by walking the list of all
planes in the mode and finding the position of that plane in the list. A
linear walk, especially a linear walk within a linear walk as frequently
conceived by i915.ko [O(N^2)] quickly comes to dominate profiles.

The plane's index is constant for as long as no earlier planes are
removed from the list. For most drivers, planes are static, determined
at boot and then untouched until shutdown. Storing the index upon
construction and then only walking the tail upon removal should
be a major improvement for all.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 38 +++++++++-----------------------------
 include/drm/drm_crtc.h     |  6 +++++-
 2 files changed, 14 insertions(+), 30 deletions(-)

Comments

Ville Syrjala May 26, 2016, 11:27 a.m. UTC | #1
On Thu, May 26, 2016 at 10:34:57AM +0100, Chris Wilson wrote:
> Currently the plane's index is determined by walking the list of all
> planes in the mode and finding the position of that plane in the list. A
> linear walk, especially a linear walk within a linear walk as frequently
> conceived by i915.ko [O(N^2)] quickly comes to dominate profiles.
> 
> The plane's index is constant for as long as no earlier planes are
> removed from the list. For most drivers, planes are static, determined
> at boot and then untouched until shutdown. Storing the index upon
> construction and then only walking the tail upon removal should
> be a major improvement for all.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Matt Roper <matthew.d.roper@intel.com>

I've been wondering about the cost of these silly walks myself.

Patch looks sane to me
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Same for crtcs?

> ---
>  drivers/gpu/drm/drm_crtc.c | 38 +++++++++-----------------------------
>  include/drm/drm_crtc.h     |  6 +++++-
>  2 files changed, 14 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3a0384cce4a2..00ee01126b6f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1302,7 +1302,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  	plane->type = type;
>  
>  	list_add_tail(&plane->head, &config->plane_list);
> -	config->num_total_plane++;
> +	plane->index = config->num_total_plane++;
>  	if (plane->type == DRM_PLANE_TYPE_OVERLAY)
>  		config->num_overlay_plane++;
>  
> @@ -1369,6 +1369,7 @@ EXPORT_SYMBOL(drm_plane_init);
>  void drm_plane_cleanup(struct drm_plane *plane)
>  {
>  	struct drm_device *dev = plane->dev;
> +	struct drm_plane *other;
>  
>  	drm_modeset_lock_all(dev);
>  	kfree(plane->format_types);
> @@ -1376,6 +1377,10 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  
>  	BUG_ON(list_empty(&plane->head));
>  
> +	other = list_next_entry(plane, head);
> +	list_for_each_entry_from(other, &dev->mode_config.plane_list, head)
> +		other->index--;
> +
>  	list_del(&plane->head);
>  	dev->mode_config.num_total_plane--;
>  	if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> @@ -1393,29 +1398,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  EXPORT_SYMBOL(drm_plane_cleanup);
>  
>  /**
> - * drm_plane_index - find the index of a registered plane
> - * @plane: plane to find index for
> - *
> - * Given a registered plane, return the index of that CRTC within a DRM
> - * device's list of planes.
> - */
> -unsigned int drm_plane_index(struct drm_plane *plane)
> -{
> -	unsigned int index = 0;
> -	struct drm_plane *tmp;
> -
> -	drm_for_each_plane(tmp, plane->dev) {
> -		if (tmp == plane)
> -			return index;
> -
> -		index++;
> -	}
> -
> -	BUG();
> -}
> -EXPORT_SYMBOL(drm_plane_index);
> -
> -/**
>   * drm_plane_from_index - find the registered plane at an index
>   * @dev: DRM device
>   * @idx: index of registered plane to find for
> @@ -1427,13 +1409,11 @@ struct drm_plane *
>  drm_plane_from_index(struct drm_device *dev, int idx)
>  {
>  	struct drm_plane *plane;
> -	unsigned int i = 0;
>  
> -	drm_for_each_plane(plane, dev) {
> -		if (i == idx)
> +	drm_for_each_plane(plane, dev)
> +		if (idx == plane->index)
>  			return plane;
> -		i++;
> -	}
> +
>  	return NULL;
>  }
>  EXPORT_SYMBOL(drm_plane_from_index);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 9771428e1ba8..eda3b1b3d3b4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1543,6 +1543,7 @@ struct drm_plane {
>  	struct drm_object_properties properties;
>  
>  	enum drm_plane_type type;
> +	unsigned index;
>  
>  	const struct drm_plane_helper_funcs *helper_private;
>  
> @@ -2316,7 +2317,10 @@ extern int drm_plane_init(struct drm_device *dev,
>  			  const uint32_t *formats, unsigned int format_count,
>  			  bool is_primary);
>  extern void drm_plane_cleanup(struct drm_plane *plane);
> -extern unsigned int drm_plane_index(struct drm_plane *plane);
> +static inline unsigned int drm_plane_index(struct drm_plane *plane)
> +{
> +	return plane->index;
> +}
>  extern struct drm_plane * drm_plane_from_index(struct drm_device *dev, int idx);
>  extern void drm_plane_force_disable(struct drm_plane *plane);
>  extern int drm_plane_check_pixel_format(const struct drm_plane *plane,
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3a0384cce4a2..00ee01126b6f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1302,7 +1302,7 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 	plane->type = type;
 
 	list_add_tail(&plane->head, &config->plane_list);
-	config->num_total_plane++;
+	plane->index = config->num_total_plane++;
 	if (plane->type == DRM_PLANE_TYPE_OVERLAY)
 		config->num_overlay_plane++;
 
@@ -1369,6 +1369,7 @@  EXPORT_SYMBOL(drm_plane_init);
 void drm_plane_cleanup(struct drm_plane *plane)
 {
 	struct drm_device *dev = plane->dev;
+	struct drm_plane *other;
 
 	drm_modeset_lock_all(dev);
 	kfree(plane->format_types);
@@ -1376,6 +1377,10 @@  void drm_plane_cleanup(struct drm_plane *plane)
 
 	BUG_ON(list_empty(&plane->head));
 
+	other = list_next_entry(plane, head);
+	list_for_each_entry_from(other, &dev->mode_config.plane_list, head)
+		other->index--;
+
 	list_del(&plane->head);
 	dev->mode_config.num_total_plane--;
 	if (plane->type == DRM_PLANE_TYPE_OVERLAY)
@@ -1393,29 +1398,6 @@  void drm_plane_cleanup(struct drm_plane *plane)
 EXPORT_SYMBOL(drm_plane_cleanup);
 
 /**
- * drm_plane_index - find the index of a registered plane
- * @plane: plane to find index for
- *
- * Given a registered plane, return the index of that CRTC within a DRM
- * device's list of planes.
- */
-unsigned int drm_plane_index(struct drm_plane *plane)
-{
-	unsigned int index = 0;
-	struct drm_plane *tmp;
-
-	drm_for_each_plane(tmp, plane->dev) {
-		if (tmp == plane)
-			return index;
-
-		index++;
-	}
-
-	BUG();
-}
-EXPORT_SYMBOL(drm_plane_index);
-
-/**
  * drm_plane_from_index - find the registered plane at an index
  * @dev: DRM device
  * @idx: index of registered plane to find for
@@ -1427,13 +1409,11 @@  struct drm_plane *
 drm_plane_from_index(struct drm_device *dev, int idx)
 {
 	struct drm_plane *plane;
-	unsigned int i = 0;
 
-	drm_for_each_plane(plane, dev) {
-		if (i == idx)
+	drm_for_each_plane(plane, dev)
+		if (idx == plane->index)
 			return plane;
-		i++;
-	}
+
 	return NULL;
 }
 EXPORT_SYMBOL(drm_plane_from_index);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 9771428e1ba8..eda3b1b3d3b4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1543,6 +1543,7 @@  struct drm_plane {
 	struct drm_object_properties properties;
 
 	enum drm_plane_type type;
+	unsigned index;
 
 	const struct drm_plane_helper_funcs *helper_private;
 
@@ -2316,7 +2317,10 @@  extern int drm_plane_init(struct drm_device *dev,
 			  const uint32_t *formats, unsigned int format_count,
 			  bool is_primary);
 extern void drm_plane_cleanup(struct drm_plane *plane);
-extern unsigned int drm_plane_index(struct drm_plane *plane);
+static inline unsigned int drm_plane_index(struct drm_plane *plane)
+{
+	return plane->index;
+}
 extern struct drm_plane * drm_plane_from_index(struct drm_device *dev, int idx);
 extern void drm_plane_force_disable(struct drm_plane *plane);
 extern int drm_plane_check_pixel_format(const struct drm_plane *plane,