From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 5/9] [v5] drm/i915/bdw: Reorganize PT allocations Date: Mon, 24 Feb 2014 15:38:40 -0800 Message-ID: <20140224233838.GA15213@bwidawsk.net> References: <1392876349-24684-6-git-send-email-benjamin.widawsky@intel.com> <1392925881-6665-1-git-send-email-benjamin.widawsky@intel.com> <1393261392.13131.126.camel@intelbox> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 189D8FAC7A for ; Mon, 24 Feb 2014 15:38:58 -0800 (PST) Content-Disposition: inline In-Reply-To: <1393261392.13131.126.camel@intelbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Imre Deak Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Mon, Feb 24, 2014 at 07:03:12PM +0200, Imre Deak wrote: > On Thu, 2014-02-20 at 11:51 -0800, Ben Widawsky wrote: > > The previous allocation mechanism would get 2 contiguous allocations, > > one for the page directories, and one for the page tables. As each page > > table is 1 page, and there are 512 of these per page directory, this > > goes to 2MB. An unfriendly request at best. Worse still, our HW now > > supports 4 page directories, and a 2MB allocation is not allowed. > > > > In order to fix this, this patch attempts to split up each page table > > allocation into a single, discrete allocation. There is nothing really > > fancy about the patch itself, it just has to manage an extra pointer > > indirection, and have a fancier bit of logic to free up the pages. > > > > To accommodate some of the added complexity, two new helpers are > > introduced to allocate, and free the page table pages. > > > > NOTE: I really wanted to split the way we do allocations, and the way in > > which we identify the page table/page directory being used. I found > > splitting this functionality up to be too unwieldy. I apologize in > > advance to the reviewer. I'd recommend looking at the result, rather > > than the diff. > > > > v2/NOTE2: This patch predated commit: > > 6f1cc993518462ccf039e195fabd47e7aa5bfd13 > > Author: Chris Wilson > > Date: Tue Dec 31 15:50:31 2013 +0000 > > > > drm/i915: Avoid dereference past end of page arr > > > > It fixed the same issue as that patch, but because of the limbo state of > > PPGTT, Chris patch was merged instead. The excess churn is a result of > > my using my original patch, which has my preferred naming. Primarily > > act_* is changed to which_*, but it's mostly the same otherwise. I've > > kept the convention Chris used for the pte wrap (I had something > > slightly different, and broken - but fixable) > > > > v3: Rename which_p[..]e to drop which_ (Chris) > > Remove BUG_ON in inner loop (Chris) > > Redo the pde/pdpe wrap logic (Chris) > > > > v4: s/1MB/2MB in commit message (Imre) > > Plug leaking gen8_pt_pages in both the error path, as well as general > > free case (Imre) > > > > v5: Rename leftover "which_" variables (Imre) > > Add the pde = 0 wrap that was missed from v3 (Imre) > > > > Reviewed-by: Imre Deak > > Signed-off-by: Ben Widawsky > > Reviewed-by: Imre Deak > Thanks very much for your thorough review of the series. -- Ben Widawsky, Intel Open Source Technology Center