From patchwork Tue Oct 21 10:47:29 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dheeraj Jamwal X-Patchwork-Id: 5115491 Return-Path: X-Original-To: patchwork-ltsi-dev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 627C8C11AC for ; Tue, 21 Oct 2014 11:18:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 56F052010F for ; Tue, 21 Oct 2014 11:18:58 +0000 (UTC) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2EDD7200FE for ; Tue, 21 Oct 2014 11:18:57 +0000 (UTC) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 6ADCAD68; Tue, 21 Oct 2014 11:02:14 +0000 (UTC) X-Original-To: ltsi-dev@lists.linuxfoundation.org Delivered-To: ltsi-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id C5FBED5C for ; Tue, 21 Oct 2014 11:02:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by smtp1.linuxfoundation.org (Postfix) with ESMTP id 0A9391FAB2 for ; Tue, 21 Oct 2014 11:02:12 +0000 (UTC) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 21 Oct 2014 04:02:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,761,1406617200"; d="scan'208";a="617793909" Received: from ubuntu-desktop.png.intel.com ([10.221.122.25]) by fmsmga002.fm.intel.com with ESMTP; 21 Oct 2014 04:02:10 -0700 From: Dheeraj Jamwal To: ltsi-dev@lists.linuxfoundation.org Date: Tue, 21 Oct 2014 18:47:29 +0800 Message-Id: <1413889294-31328-250-git-send-email-dheerajx.s.jamwal@intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1413889294-31328-1-git-send-email-dheerajx.s.jamwal@intel.com> References: <1413889294-31328-1-git-send-email-dheerajx.s.jamwal@intel.com> X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org Subject: [LTSI-dev] [PATCH 0249/1094] drm/i915/bdw: Reorganize PPGTT init X-BeenThere: ltsi-dev@lists.linuxfoundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: "A list to discuss patches, development, and other things related to the LTSI project" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ltsi-dev-bounces@lists.linuxfoundation.org Errors-To: ltsi-dev-bounces@lists.linuxfoundation.org X-Virus-Scanned: ClamAV using ClamSMTP From: Ben Widawsky Create 3 clear stages in PPGTT init. This will help with upcoming changes be more readable. The 3 stages are, allocation, dma mapping, and writing the P[DT]Es One nice benefit to the patches is that it makes 2 very clear error points, allocation, and mapping, and avoids having to do any handling after writing PTEs (something which was likely buggy before). This simplified error handling I suspect will be helpful when we move to deferred/dynamic page table allocation and mapping. The patches also attempts to break up some of the steps into more logical reviewable chunks, particularly when we free. v2: Don't call cleanup on the error path since that takes down the drm_mm and list entry, which aren't setup at this point. v3: Fixes addressing Imre's comments from: <1392821989.19792.13.camel@intelbox> Don't do dynamic allocation for the page table DMA addresses. I can't remember why I did it in the first place. This addresses one of Imre's other issues. Fix error path leak of page tables. v4: Fix the fix of the error path leak. Original fix still leaked page tables. (Imre) Reviewed-by: Imre Deak Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter (cherry picked from commit f3a964b96d792eac67976a3bb296a0d2049b4f91) Signed-off-by: Dheeraj Jamwal --- drivers/gpu/drm/i915/i915_gem_gtt.c | 110 +++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 8da049a..4f73e137 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -332,6 +332,7 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt) static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) { + struct pci_dev *hwdev = ppgtt->base.dev->pdev; int i, j; for (i = 0; i < ppgtt->num_pd_pages; i++) { @@ -340,18 +341,14 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) if (!ppgtt->pd_dma_addr[i]) continue; - pci_unmap_page(ppgtt->base.dev->pdev, - ppgtt->pd_dma_addr[i], - PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); + pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i], PAGE_SIZE, + PCI_DMA_BIDIRECTIONAL); for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j]; if (addr) - pci_unmap_page(ppgtt->base.dev->pdev, - addr, - PAGE_SIZE, - PCI_DMA_BIDIRECTIONAL); - + pci_unmap_page(hwdev, addr, PAGE_SIZE, + PCI_DMA_BIDIRECTIONAL); } } } @@ -369,27 +366,27 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) } /** - * GEN8 legacy ppgtt programming is accomplished through 4 PDP registers with a - * net effect resembling a 2-level page table in normal x86 terms. Each PDP - * represents 1GB of memory - * 4 * 512 * 512 * 4096 = 4GB legacy 32b address space. + * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers + * with a net effect resembling a 2-level page table in normal x86 terms. Each + * PDP represents 1GB of memory 4 * 512 * 512 * 4096 = 4GB legacy 32b address + * space. * + * FIXME: split allocation into smaller pieces. For now we only ever do this + * once, but with full PPGTT, the multiple contiguous allocations will be bad. * TODO: Do something with the size parameter - **/ + */ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) { struct page *pt_pages; - int i, j, ret = -ENOMEM; const int max_pdp = DIV_ROUND_UP(size, 1 << 30); const int num_pt_pages = GEN8_PDES_PER_PAGE * max_pdp; + struct pci_dev *hwdev = ppgtt->base.dev->pdev; + int i, j, ret; if (size % (1<<30)) DRM_INFO("Pages will be wasted unless GTT size (%llu) is divisible by 1GB\n", size); - /* FIXME: split allocation into smaller pieces. For now we only ever do - * this once, but with full PPGTT, the multiple contiguous allocations - * will be bad. - */ + /* 1. Do all our allocations for page directories and page tables */ ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT)); if (!ppgtt->pd_pages) return -ENOMEM; @@ -404,52 +401,56 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT); ppgtt->num_pt_pages = 1 << get_order(num_pt_pages << PAGE_SHIFT); ppgtt->num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE; - ppgtt->enable = gen8_ppgtt_enable; - ppgtt->switch_mm = gen8_mm_switch; - ppgtt->base.clear_range = gen8_ppgtt_clear_range; - ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; - ppgtt->base.cleanup = gen8_ppgtt_cleanup; - ppgtt->base.start = 0; - ppgtt->base.total = ppgtt->num_pt_pages * GEN8_PTES_PER_PAGE * PAGE_SIZE; - BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS); + for (i = 0; i < max_pdp; i++) { + ppgtt->gen8_pt_dma_addr[i] = kcalloc(GEN8_PDES_PER_PAGE, + sizeof(dma_addr_t), + GFP_KERNEL); + if (!ppgtt->gen8_pt_dma_addr[i]) { + ret = -ENOMEM; + goto bail; + } + } + /* - * - Create a mapping for the page directories. - * - For each page directory: - * allocate space for page table mappings. - * map each page table + * 2. Create all the DMA mappings for the page directories and page + * tables */ for (i = 0; i < max_pdp; i++) { - dma_addr_t temp; - temp = pci_map_page(ppgtt->base.dev->pdev, - &ppgtt->pd_pages[i], 0, - PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); - if (pci_dma_mapping_error(ppgtt->base.dev->pdev, temp)) - goto err_out; + dma_addr_t pd_addr, pt_addr; - ppgtt->pd_dma_addr[i] = temp; + /* Get the page directory mappings */ + pd_addr = pci_map_page(hwdev, &ppgtt->pd_pages[i], 0, + PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); + ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr); + if (ret) + goto bail; - ppgtt->gen8_pt_dma_addr[i] = kmalloc(sizeof(dma_addr_t) * GEN8_PDES_PER_PAGE, GFP_KERNEL); - if (!ppgtt->gen8_pt_dma_addr[i]) - goto err_out; + ppgtt->pd_dma_addr[i] = pd_addr; + /* And the page table mappings per page directory */ for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { struct page *p = &pt_pages[i * GEN8_PDES_PER_PAGE + j]; - temp = pci_map_page(ppgtt->base.dev->pdev, - p, 0, PAGE_SIZE, - PCI_DMA_BIDIRECTIONAL); - if (pci_dma_mapping_error(ppgtt->base.dev->pdev, temp)) - goto err_out; + pt_addr = pci_map_page(hwdev, p, 0, PAGE_SIZE, + PCI_DMA_BIDIRECTIONAL); + ret = pci_dma_mapping_error(hwdev, pt_addr); + if (ret) + goto bail; - ppgtt->gen8_pt_dma_addr[i][j] = temp; + ppgtt->gen8_pt_dma_addr[i][j] = pt_addr; } } - /* For now, the PPGTT helper functions all require that the PDEs are + /* + * 3. Map all the page directory entires to point to the page tables + * we've allocated. + * + * For now, the PPGTT helper functions all require that the PDEs are * plugged in correctly. So we do that now/here. For aliasing PPGTT, we - * will never need to touch the PDEs again */ + * will never need to touch the PDEs again. + */ for (i = 0; i < max_pdp; i++) { gen8_ppgtt_pde_t *pd_vaddr; pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]); @@ -461,6 +462,14 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) kunmap_atomic(pd_vaddr); } + ppgtt->enable = gen8_ppgtt_enable; + ppgtt->switch_mm = gen8_mm_switch; + ppgtt->base.clear_range = gen8_ppgtt_clear_range; + ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; + ppgtt->base.cleanup = gen8_ppgtt_cleanup; + ppgtt->base.start = 0; + ppgtt->base.total = ppgtt->num_pt_pages * GEN8_PTES_PER_PAGE * PAGE_SIZE; + ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE, true); @@ -473,8 +482,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) size % (1<<30)); return 0; -err_out: - ppgtt->base.cleanup(&ppgtt->base); +bail: + gen8_ppgtt_unmap_pages(ppgtt); + gen8_ppgtt_free(ppgtt); return ret; }