From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 3/5] drm/i915: Extract gen6 aliasing ppgtt code Date: Sat, 29 Dec 2012 11:49:01 +0000 Message-ID: <84c8a8$74vpth@orsmga001.jf.intel.com> References: <1356755249-19810-1-git-send-email-ben@bwidawsk.net> <1356755249-19810-3-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 9786FE5C2F for ; Sat, 29 Dec 2012 03:49:15 -0800 (PST) In-Reply-To: <1356755249-19810-3-git-send-email-ben@bwidawsk.net> 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: intel-gfx@lists.freedesktop.org Cc: Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Fri, 28 Dec 2012 20:27:27 -0800, Ben Widawsky wrote: > This refactor clearly identifies the GEN specific portion of the aliased > ppgtt code. Aside from some of the gen specific parts being extracted, > it also preps us for an upcoming patch that pulls out the size, which > has some nice benefits. Error handling looks a little wonky. > > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 51 ++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 94d21aa..3b1bff1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -108,29 +108,20 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt, > } > } > > -static int i915_gem_init_aliasing_ppgtt(struct drm_device *dev) > +static int gen6_init_aliasing_ppgtt(struct i915_hw_ppgtt *ppgtt) > { > + struct drm_device *dev = ppgtt->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct i915_hw_ppgtt *ppgtt; > unsigned first_pd_entry_in_global_pt; > - int i; > - int ret = -ENOMEM; > + int i, ret = -ENOMEM; > > /* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024 > * entries. For aliasing ppgtt support we just steal them at the end for > * now. */ > first_pd_entry_in_global_pt = dev_priv->mm.gtt->gtt_total_entries - I915_PPGTT_PD_ENTRIES; > - > - ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL); > - if (!ppgtt) > - return ret; > - > - ppgtt->dev = dev; > ppgtt->num_pd_entries = I915_PPGTT_PD_ENTRIES; > ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries, > GFP_KERNEL); > - if (!ppgtt->pt_pages) > - goto err_ppgtt; I think you still want this... > @@ -185,9 +171,32 @@ err_pt_alloc: > if (ppgtt->pt_pages[i]) > __free_page(ppgtt->pt_pages[i]); > } > - kfree(ppgtt->pt_pages); > -err_ppgtt: > - kfree(ppgtt); > + > + if (!ppgtt->pt_pages) > + kfree(ppgtt->pt_pages); And will want to reconsider this chunk. -Chris -- Chris Wilson, Intel Open Source Technology Centre