Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

2013-07-01 Thread David Herrmann 
Hi

On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 When converting to the preallocated drm_mm_node interfaces in

 commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Fri Dec 7 20:37:07 2012 +

 drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm

 only the allocation side was converted, but not the freeing. Fix this
 up.

 Note that the only difference between put_block and remove_node is
 that the former fills up the preallocation cache. Which we don't need
 anyway and hence is just wasted space.

 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 4200c32..30fd694 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 /* Avoid an unnecessary call to unbind on rebind. */
 obj-map_and_fenceable = true;

 -   drm_mm_put_block(obj-gtt_space);
 +   drm_mm_remove_node(obj-gtt_space);

kfree(obj-gtt_space);

 obj-gtt_space = NULL;
 obj-gtt_offset = 0;

 @@ -3137,14 +3137,14 @@ search_free:
 }
 if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj-cache_level))) {
 i915_gem_object_unpin_pages(obj);
 -   drm_mm_put_block(node);
 +   drm_mm_remove_node(node);

kfree(node);

 return -EINVAL;
 }

 ret = i915_gem_gtt_prepare_object(obj);
 if (ret) {
 i915_gem_object_unpin_pages(obj);
 -   drm_mm_put_block(node);
 +   drm_mm_remove_node(node);

kfree(node);

drm_mm_remove_node() does unlink the node but not remove it. Btw., I
have these fixes in my series, too. I will push it later and write the
git-link to #dri-devel.

Cheers
David

 return ret;
 }

 --
 1.7.11.7

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

2013-07-01 Thread Daniel Vetter 
On Mon, Jul 01, 2013 at 09:16:56PM +0100, Chris Wilson wrote:
 On Mon, Jul 01, 2013 at 10:05:54PM +0200, Daniel Vetter wrote:
  When converting to the preallocated drm_mm_node interfaces in
  
  commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
  Author: Chris Wilson ch...@chris-wilson.co.uk
  Date:   Fri Dec 7 20:37:07 2012 +
  
  drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
  drm_mm
  
  only the allocation side was converted, but not the freeing. Fix this
  up.
  
  Note that the only difference between put_block and remove_node is
  that the former fills up the preallocation cache. Which we don't need
  anyway and hence is just wasted space.
  
  Cc: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky b...@bwidawsk.net
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 
 You learn something new every day.
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk

The lack of kfree in this patch might be a problem ...

/me tries again
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

2013-07-01 Thread Daniel Vetter 
On Mon, Jul 01, 2013 at 10:21:57PM +0200, David Herrmann wrote:
 Hi
 
 On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  When converting to the preallocated drm_mm_node interfaces in
 
  commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
  Author: Chris Wilson ch...@chris-wilson.co.uk
  Date:   Fri Dec 7 20:37:07 2012 +
 
  drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
  drm_mm
 
  only the allocation side was converted, but not the freeing. Fix this
  up.
 
  Note that the only difference between put_block and remove_node is
  that the former fills up the preallocation cache. Which we don't need
  anyway and hence is just wasted space.
 
  Cc: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky b...@bwidawsk.net
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_gem.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index 4200c32..30fd694 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
  *obj)
  /* Avoid an unnecessary call to unbind on rebind. */
  obj-map_and_fenceable = true;
 
  -   drm_mm_put_block(obj-gtt_space);
  +   drm_mm_remove_node(obj-gtt_space);
 
 kfree(obj-gtt_space);
 
  obj-gtt_space = NULL;
  obj-gtt_offset = 0;
 
  @@ -3137,14 +3137,14 @@ search_free:
  }
  if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, 
  obj-cache_level))) {
  i915_gem_object_unpin_pages(obj);
  -   drm_mm_put_block(node);
  +   drm_mm_remove_node(node);
 
 kfree(node);
 
  return -EINVAL;
  }
 
  ret = i915_gem_gtt_prepare_object(obj);
  if (ret) {
  i915_gem_object_unpin_pages(obj);
  -   drm_mm_put_block(node);
  +   drm_mm_remove_node(node);
 
 kfree(node);

Yeah, I fail ...

 drm_mm_remove_node() does unlink the node but not remove it. Btw., I
 have these fixes in my series, too. I will push it later and write the
 git-link to #dri-devel.

We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
so I guess that part will solve itself automatically.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

2013-07-01 Thread Ben Widawsky 
On Mon, Jul 01, 2013 at 10:39:03PM +0200, Daniel Vetter wrote:
 On Mon, Jul 01, 2013 at 10:21:57PM +0200, David Herrmann wrote:
  Hi
  
  On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch 
  wrote:
   When converting to the preallocated drm_mm_node interfaces in
  
   commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
   Author: Chris Wilson ch...@chris-wilson.co.uk
   Date:   Fri Dec 7 20:37:07 2012 +
  
   drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
   drm_mm
  
   only the allocation side was converted, but not the freeing. Fix this
   up.
  
   Note that the only difference between put_block and remove_node is
   that the former fills up the preallocation cache. Which we don't need
   anyway and hence is just wasted space.
  
   Cc: Chris Wilson ch...@chris-wilson.co.uk
   Cc: Ben Widawsky b...@bwidawsk.net
   Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
   ---
drivers/gpu/drm/i915/i915_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
  
   diff --git a/drivers/gpu/drm/i915/i915_gem.c 
   b/drivers/gpu/drm/i915/i915_gem.c
   index 4200c32..30fd694 100644
   --- a/drivers/gpu/drm/i915/i915_gem.c
   +++ b/drivers/gpu/drm/i915/i915_gem.c
   @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
   *obj)
   /* Avoid an unnecessary call to unbind on rebind. */
   obj-map_and_fenceable = true;
  
   -   drm_mm_put_block(obj-gtt_space);
   +   drm_mm_remove_node(obj-gtt_space);
  
  kfree(obj-gtt_space);
  
   obj-gtt_space = NULL;
   obj-gtt_offset = 0;
  
   @@ -3137,14 +3137,14 @@ search_free:
   }
   if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, 
   obj-cache_level))) {
   i915_gem_object_unpin_pages(obj);
   -   drm_mm_put_block(node);
   +   drm_mm_remove_node(node);
  
  kfree(node);
  
   return -EINVAL;
   }
  
   ret = i915_gem_gtt_prepare_object(obj);
   if (ret) {
   i915_gem_object_unpin_pages(obj);
   -   drm_mm_put_block(node);
   +   drm_mm_remove_node(node);
  
  kfree(node);
 
 Yeah, I fail ...
 
  drm_mm_remove_node() does unlink the node but not remove it. Btw., I
  have these fixes in my series, too. I will push it later and write the
  git-link to #dri-devel.
 
 We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
 so I guess that part will solve itself automatically.
 -Daniel

I'm planning to get this out ASAP. I'm a bit confused now though what I
actually need to send.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

2013-07-01 Thread Daniel Vetter 
On Mon, Jul 1, 2013 at 10:46 PM, Ben Widawsky b...@bwidawsk.net wrote:
  drm_mm_remove_node() does unlink the node but not remove it. Btw., I
  have these fixes in my series, too. I will push it later and write the
  git-link to #dri-devel.

 We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
 so I guess that part will solve itself automatically.

 I'm planning to get this out ASAP. I'm a bit confused now though what I
 actually need to send.

I think for now just the top-down allocator + the create_node stuff
for stolen memory. I guess the conversion to embed the gtt_space
drm_mm_node will take a bit longer.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel