diff mbox

[1/3] drm: Export routines for inserting preallocated nodes into the mm manager

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

Commit Message

Chris Wilson Dec. 7, 2012, 8:37 p.m. UTC 
Required by i915 in order to avoid the allocation in the middle of
manipulating the drm_mm lists.

Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
backporting; to be removed later.

Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
 include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 16 deletions(-)

Comments

Jani Nikula Dec. 12, 2012, 10:02 a.m. UTC | #1 
On Fri, 07 Dec 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Required by i915 in order to avoid the allocation in the middle of
> manipulating the drm_mm lists.
>
> Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
> backporting; to be removed later.

Regardless of whether you choose to do anything about the two nitpicks
below or not,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
>  include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 0761a03..d93dc05 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -184,19 +184,27 @@ EXPORT_SYMBOL(drm_mm_get_block_generic);
>   * -ENOSPC if no suitable free area is available. The preallocated memory node
>   * must be cleared.
>   */
> -int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -		       unsigned long size, unsigned alignment)
> +int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +			       unsigned long size, unsigned alignment,
> +			       unsigned long color, int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free(mm, size, alignment, false);
> +	hole_node = drm_mm_search_free_generic(mm, size, alignment,
> +					       color, atomic);

The last param for drm_mm_search_free_generic is a bool, any reason not
to make atomic a bool too?

>  	if (!hole_node)
>  		return -ENOSPC;
>  
> -	drm_mm_insert_helper(hole_node, node, size, alignment, 0);
> -
> +	drm_mm_insert_helper(hole_node, node, size, alignment, color);
>  	return 0;
>  }
> +EXPORT_SYMBOL(drm_mm_insert_node_generic);
> +
> +int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> +		       unsigned long size, unsigned alignment)
> +{
> +	return drm_mm_insert_node_generic(mm, node, size, alignment, 0, false);
> +}
>  EXPORT_SYMBOL(drm_mm_insert_node);
>  
>  static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node,
> @@ -275,22 +283,32 @@ EXPORT_SYMBOL(drm_mm_get_block_range_generic);
>   * -ENOSPC if no suitable free area is available. This is for range
>   * restricted allocations. The preallocated memory node must be cleared.
>   */
> -int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
> -				unsigned long size, unsigned alignment,
> -				unsigned long start, unsigned long end)
> +int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +					unsigned long size, unsigned alignment, unsigned long color,
> +					unsigned long start, unsigned long end,
> +					int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free_in_range(mm, size, alignment,
> -						start, end, false);
> +	hole_node = drm_mm_search_free_in_range_generic(mm,
> +							size, alignment, color,
> +							start, end, atomic);

Same as above.

>  	if (!hole_node)
>  		return -ENOSPC;
>  
> -	drm_mm_insert_helper_range(hole_node, node, size, alignment, 0,
> +	drm_mm_insert_helper_range(hole_node, node,
> +				   size, alignment, color,
>  				   start, end);
> -
>  	return 0;
>  }
> +EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
> +
> +int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
> +				unsigned long size, unsigned alignment,
> +				unsigned long start, unsigned long end)
> +{
> +	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end, false);
> +}
>  EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>  
>  /**
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index 06d7f79..87b0968 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -158,12 +158,31 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
>  	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
>  						start, end, 1);
>  }
> -extern int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -			      unsigned long size, unsigned alignment);
> +
> +extern int drm_mm_insert_node(struct drm_mm *mm,
> +			      struct drm_mm_node *node,
> +			      unsigned long size,
> +			      unsigned alignment);
>  extern int drm_mm_insert_node_in_range(struct drm_mm *mm,
>  				       struct drm_mm_node *node,
> -				       unsigned long size, unsigned alignment,
> -				       unsigned long start, unsigned long end);
> +				       unsigned long size,
> +				       unsigned alignment,
> +				       unsigned long start,
> +				       unsigned long end);
> +extern int drm_mm_insert_node_generic(struct drm_mm *mm,
> +				      struct drm_mm_node *node,
> +				      unsigned long size,
> +				      unsigned alignment,
> +				      unsigned long color,
> +				      int atomic);
> +extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm,
> +				       struct drm_mm_node *node,
> +				       unsigned long size,
> +				       unsigned alignment,
> +				       unsigned long color,
> +				       unsigned long start,
> +				       unsigned long end,
> +				       int atomic);
>  extern void drm_mm_put_block(struct drm_mm_node *cur);
>  extern void drm_mm_remove_node(struct drm_mm_node *node);
>  extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new);
> -- 
> 1.7.10.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter Dec. 18, 2012, 12:28 a.m. UTC | #2 
On Fri, Dec 07, 2012 at 08:37:06PM +0000, Chris Wilson wrote:
> Required by i915 in order to avoid the allocation in the middle of
> manipulating the drm_mm lists.
> 
> Use a pair of stubs to preserve the existing EXPORT_SYMBOLs for
> backporting; to be removed later.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_mm.c |   42 ++++++++++++++++++++++++++++++------------
>  include/drm/drm_mm.h     |   27 +++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 0761a03..d93dc05 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -184,19 +184,27 @@ EXPORT_SYMBOL(drm_mm_get_block_generic);
>   * -ENOSPC if no suitable free area is available. The preallocated memory node
>   * must be cleared.
>   */
> -int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
> -		       unsigned long size, unsigned alignment)
> +int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
> +			       unsigned long size, unsigned alignment,
> +			       unsigned long color, int atomic)
>  {
>  	struct drm_mm_node *hole_node;
>  
> -	hole_node = drm_mm_search_free(mm, size, alignment, false);
> +	hole_node = drm_mm_search_free_generic(mm, size, alignment,
> +					       color, atomic);

The last parameter of search_free_generic is best_match, which isn't used
by any current caller. The only reason it's still there is that I haven't
converted yet all drm_mm.c users to preallocate drm_mm_node's, but as soon
as that's done best_match will die together with search_free_generic as a
public interface.

So what's the atomic doing in here? I've looked through the drm/i915
patches and couldn't see any reason ... Can we just respin the two i915
patches with _generic and atomic = false dropped, or do I miss something
big here?
-Daniel
Chris Wilson Dec. 18, 2012, 9:13 a.m. UTC | #3 
On Tue, 18 Dec 2012 01:28:22 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> The last parameter of search_free_generic is best_match, which isn't used
> by any current caller. The only reason it's still there is that I haven't
> converted yet all drm_mm.c users to preallocate drm_mm_node's, but as soon
> as that's done best_match will die together with search_free_generic as a
> public interface.
> 
> So what's the atomic doing in here? I've looked through the drm/i915
> patches and couldn't see any reason ... Can we just respin the two i915
> patches with _generic and atomic = false dropped, or do I miss something
> big here?

It is scheduled to become a flags parameter for doing more funky
searches. (Allocating preferrentially from unmappable for LLC/snoopable
buffers etc) But the whole idea of the _generic functions is that they
do no currying at all, otherwise we will eventually need a _generic_full.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 0761a03..d93dc05 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -184,19 +184,27 @@  EXPORT_SYMBOL(drm_mm_get_block_generic);
  * -ENOSPC if no suitable free area is available. The preallocated memory node
  * must be cleared.
  */
-int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
-		       unsigned long size, unsigned alignment)
+int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
+			       unsigned long size, unsigned alignment,
+			       unsigned long color, int atomic)
 {
 	struct drm_mm_node *hole_node;
 
-	hole_node = drm_mm_search_free(mm, size, alignment, false);
+	hole_node = drm_mm_search_free_generic(mm, size, alignment,
+					       color, atomic);
 	if (!hole_node)
 		return -ENOSPC;
 
-	drm_mm_insert_helper(hole_node, node, size, alignment, 0);
-
+	drm_mm_insert_helper(hole_node, node, size, alignment, color);
 	return 0;
 }
+EXPORT_SYMBOL(drm_mm_insert_node_generic);
+
+int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
+		       unsigned long size, unsigned alignment)
+{
+	return drm_mm_insert_node_generic(mm, node, size, alignment, 0, false);
+}
 EXPORT_SYMBOL(drm_mm_insert_node);
 
 static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node,
@@ -275,22 +283,32 @@  EXPORT_SYMBOL(drm_mm_get_block_range_generic);
  * -ENOSPC if no suitable free area is available. This is for range
  * restricted allocations. The preallocated memory node must be cleared.
  */
-int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
-				unsigned long size, unsigned alignment,
-				unsigned long start, unsigned long end)
+int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node,
+					unsigned long size, unsigned alignment, unsigned long color,
+					unsigned long start, unsigned long end,
+					int atomic)
 {
 	struct drm_mm_node *hole_node;
 
-	hole_node = drm_mm_search_free_in_range(mm, size, alignment,
-						start, end, false);
+	hole_node = drm_mm_search_free_in_range_generic(mm,
+							size, alignment, color,
+							start, end, atomic);
 	if (!hole_node)
 		return -ENOSPC;
 
-	drm_mm_insert_helper_range(hole_node, node, size, alignment, 0,
+	drm_mm_insert_helper_range(hole_node, node,
+				   size, alignment, color,
 				   start, end);
-
 	return 0;
 }
+EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
+
+int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node,
+				unsigned long size, unsigned alignment,
+				unsigned long start, unsigned long end)
+{
+	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end, false);
+}
 EXPORT_SYMBOL(drm_mm_insert_node_in_range);
 
 /**
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 06d7f79..87b0968 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -158,12 +158,31 @@  static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
 	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
 						start, end, 1);
 }
-extern int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node,
-			      unsigned long size, unsigned alignment);
+
+extern int drm_mm_insert_node(struct drm_mm *mm,
+			      struct drm_mm_node *node,
+			      unsigned long size,
+			      unsigned alignment);
 extern int drm_mm_insert_node_in_range(struct drm_mm *mm,
 				       struct drm_mm_node *node,
-				       unsigned long size, unsigned alignment,
-				       unsigned long start, unsigned long end);
+				       unsigned long size,
+				       unsigned alignment,
+				       unsigned long start,
+				       unsigned long end);
+extern int drm_mm_insert_node_generic(struct drm_mm *mm,
+				      struct drm_mm_node *node,
+				      unsigned long size,
+				      unsigned alignment,
+				      unsigned long color,
+				      int atomic);
+extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm,
+				       struct drm_mm_node *node,
+				       unsigned long size,
+				       unsigned alignment,
+				       unsigned long color,
+				       unsigned long start,
+				       unsigned long end,
+				       int atomic);
 extern void drm_mm_put_block(struct drm_mm_node *cur);
 extern void drm_mm_remove_node(struct drm_mm_node *node);
 extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new);