Re: [PATCH] drm: Store the plane's index

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx