From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block Date: Mon, 1 Jul 2013 13:46:09 -0700 Message-ID: <20130701204609.GI4242@bwidawsk.net> References: <1372709154-7524-1-git-send-email-daniel.vetter@ffwll.ch> <1372709154-7524-2-git-send-email-daniel.vetter@ffwll.ch> <20130701203903.GE18285@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20130701203903.GE18285@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , DRI Development , David Herrmann List-Id: dri-devel@lists.freedesktop.org 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 wrote: > > > When converting to the preallocated drm_mm_node interfaces in > > > > > > commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d > > > Author: Chris Wilson > > > Date: Fri Dec 7 20:37:07 2012 +0000 > > > > > > 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 > > > Cc: Ben Widawsky > > > Signed-off-by: Daniel Vetter > > > --- > > > 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