| Message ID | 1421946106-7921-6-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Michel Thierry <michel.thierry@intel.com> writes: > From: Ben Widawsky <benjamin.widawsky@intel.com> > > When we move to dynamic page allocation, keeping page_directory and pagetabs as > separate structures will help to break actions into simpler tasks. > > To help transition the code nicely there is some wasted space in gen6/7. > This will be ameliorated shortly. > > Following the x86 pagetable terminology: > PDPE = struct i915_page_directory_pointer_entry. > PDE = struct i915_page_directory_entry [page_directory]. > PTE = struct i915_page_table_entry [page_tables]. > > v2: fixed mismatches after clean-up/rebase. > > v3: Clarify the names of the multiple levels of page tables (Daniel) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2, v3) > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 177 ++++++++++++++++++------------------ > drivers/gpu/drm/i915/i915_gem_gtt.h | 23 ++++- > 2 files changed, 107 insertions(+), 93 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index b48b586..98b4698 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -334,7 +334,8 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, > I915_CACHE_LLC, use_scratch); > > while (num_entries) { > - struct page *page_table = ppgtt->gen8_pt_pages[pdpe][pde]; > + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; > + struct page *page_table = pd->page_tables[pde].page; > > last_pte = pte + num_entries; > if (last_pte > GEN8_PTES_PER_PAGE) > @@ -378,8 +379,12 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, > if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES)) > break; > > - if (pt_vaddr == NULL) > - pt_vaddr = kmap_atomic(ppgtt->gen8_pt_pages[pdpe][pde]); > + if (pt_vaddr == NULL) { > + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; > + struct page *page_table = pd->page_tables[pde].page; > + > + pt_vaddr = kmap_atomic(page_table); > + } > > pt_vaddr[pte] = > gen8_pte_encode(sg_page_iter_dma_address(&sg_iter), > @@ -403,29 +408,33 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, > } > } > > -static void gen8_free_page_tables(struct page **pt_pages) > +static void gen8_free_page_tables(struct i915_page_directory_entry *pd) > { > int i; > > - if (pt_pages == NULL) > + if (pd->page_tables == NULL) > return; > > for (i = 0; i < GEN8_PDES_PER_PAGE; i++) > - if (pt_pages[i]) > - __free_pages(pt_pages[i], 0); > + if (pd->page_tables[i].page) > + __free_page(pd->page_tables[i].page); > } > > -static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt) > +static void gen8_free_page_directories(struct i915_page_directory_entry *pd) ^ You only free one directory so why plural here? > +{ If you free the page tables for the directory here.. > + kfree(pd->page_tables); > + __free_page(pd->page); > +} > + > +static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt) > { > int i; > > for (i = 0; i < ppgtt->num_pd_pages; i++) { > - gen8_free_page_tables(ppgtt->gen8_pt_pages[i]); > - kfree(ppgtt->gen8_pt_pages[i]); > + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); ...this loop will be cleaner. Also consider renaming 'num_pd_pages' to 'num_pd'. But if it does cause a lot of rebase burden dont worry about it. > + gen8_free_page_directories(&ppgtt->pdp.page_directory[i]); > kfree(ppgtt->gen8_pt_dma_addr[i]); > } > - > - __free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT)); > } > > static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) > @@ -460,86 +469,75 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) > gen8_ppgtt_free(ppgtt); > } > > -static struct page **__gen8_alloc_page_tables(void) > +static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) > { > - struct page **pt_pages; > int i; > > - pt_pages = kcalloc(GEN8_PDES_PER_PAGE, sizeof(struct page *), GFP_KERNEL); > - if (!pt_pages) > - return ERR_PTR(-ENOMEM); > - > - for (i = 0; i < GEN8_PDES_PER_PAGE; i++) { > - pt_pages[i] = alloc_page(GFP_KERNEL); > - if (!pt_pages[i]) > - goto bail; > + for (i = 0; i < ppgtt->num_pd_pages; 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]) > + return -ENOMEM; > } > > - return pt_pages; > - > -bail: > - gen8_free_page_tables(pt_pages); > - kfree(pt_pages); > - return ERR_PTR(-ENOMEM); > + return 0; > } > > -static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt, > - const int max_pdp) > +static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) > { > - struct page **pt_pages[GEN8_LEGACY_PDPES]; > - int i, ret; > + int i, j; > > - for (i = 0; i < max_pdp; i++) { > - pt_pages[i] = __gen8_alloc_page_tables(); > - if (IS_ERR(pt_pages[i])) { > - ret = PTR_ERR(pt_pages[i]); > - goto unwind_out; > + for (i = 0; i < ppgtt->num_pd_pages; i++) { > + for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { > + struct i915_page_table_entry *pt = &ppgtt->pdp.page_directory[i].page_tables[j]; > + > + pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO); > + if (!pt->page) > + goto unwind_out; > } > } > > - /* NB: Avoid touching gen8_pt_pages until last to keep the allocation, > - * "atomic" - for cleanup purposes. > - */ > - for (i = 0; i < max_pdp; i++) > - ppgtt->gen8_pt_pages[i] = pt_pages[i]; > - > return 0; > > unwind_out: > - while (i--) { > - gen8_free_page_tables(pt_pages[i]); > - kfree(pt_pages[i]); > - } > + while (i--) > + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); > > - return ret; > + return -ENOMEM; > } > > -static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) > +static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, > + const int max_pdp) > { > int i; > > - for (i = 0; i < ppgtt->num_pd_pages; 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]) > - return -ENOMEM; > - } > + for (i = 0; i < max_pdp; i++) { > + struct i915_page_table_entry *pt; > > - return 0; > -} > + pt = kcalloc(GEN8_PDES_PER_PAGE, sizeof(*pt), GFP_KERNEL); > + if (!pt) > + goto unwind_out; > > -static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, > - const int max_pdp) > -{ > - ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT)); > - if (!ppgtt->pd_pages) > - return -ENOMEM; > + ppgtt->pdp.page_directory[i].page = alloc_page(GFP_KERNEL); > + if (!ppgtt->pdp.page_directory[i].page) > + goto unwind_out; If you end up having alloc error here you will leak the previously allocated pt above. Also consider that if you do gen8_ppgtt_allocate_page_directory() and add null check for pd->page in gen8_free_page_directory you should be able to avoid the unwinding below completely. > + > + ppgtt->pdp.page_directory[i].page_tables = pt; > + } > > - ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT); > + ppgtt->num_pd_pages = max_pdp; > BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPES); > > return 0; > + > +unwind_out: > + while (i--) { > + kfree(ppgtt->pdp.page_directory[i].page_tables); > + __free_page(ppgtt->pdp.page_directory[i].page); > + } > + > + return -ENOMEM; > } > > static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, > @@ -551,18 +549,19 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, > if (ret) > return ret; > > - ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp); > - if (ret) { > - __free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT)); > - return ret; > - } > + ret = gen8_ppgtt_allocate_page_tables(ppgtt); > + if (ret) > + goto err_out; > > ppgtt->num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE; > > ret = gen8_ppgtt_allocate_dma(ppgtt); > - if (ret) > - gen8_ppgtt_free(ppgtt); > + if (!ret) > + return ret; > > + /* TODO: Check this for all cases */ The check for zero return and then returning it with the comment is confusing. Why not just do the same pattern as in above? if (ret) goto err_out; return 0; -Mika > +err_out: > + gen8_ppgtt_free(ppgtt); > return ret; > } > > @@ -573,7 +572,7 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt, > int ret; > > pd_addr = pci_map_page(ppgtt->base.dev->pdev, > - &ppgtt->pd_pages[pd], 0, > + ppgtt->pdp.page_directory[pd].page, 0, > PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); > > ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr); > @@ -593,7 +592,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt, > struct page *p; > int ret; > > - p = ppgtt->gen8_pt_pages[pd][pt]; > + p = ppgtt->pdp.page_directory[pd].page_tables[pt].page; > pt_addr = pci_map_page(ppgtt->base.dev->pdev, > p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); > ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr); > @@ -654,7 +653,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) > */ > for (i = 0; i < max_pdp; i++) { > gen8_ppgtt_pde_t *pd_vaddr; > - pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]); > + pd_vaddr = kmap_atomic(ppgtt->pdp.page_directory[i].page); > for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { > dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j]; > pd_vaddr[j] = gen8_pde_encode(ppgtt->base.dev, addr, > @@ -717,7 +716,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) > expected); > seq_printf(m, "\tPDE: %x\n", pd_entry); > > - pt_vaddr = kmap_atomic(ppgtt->pt_pages[pde]); > + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[pde].page); > for (pte = 0; pte < I915_PPGTT_PT_ENTRIES; pte+=4) { > unsigned long va = > (pde * PAGE_SIZE * I915_PPGTT_PT_ENTRIES) + > @@ -922,7 +921,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm, > if (last_pte > I915_PPGTT_PT_ENTRIES) > last_pte = I915_PPGTT_PT_ENTRIES; > > - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); > + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); > > for (i = first_pte; i < last_pte; i++) > pt_vaddr[i] = scratch_pte; > @@ -951,7 +950,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm, > pt_vaddr = NULL; > for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) { > if (pt_vaddr == NULL) > - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); > + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); > > pt_vaddr[act_pte] = > vm->pte_encode(sg_page_iter_dma_address(&sg_iter), > @@ -986,8 +985,8 @@ static void gen6_ppgtt_free(struct i915_hw_ppgtt *ppgtt) > > kfree(ppgtt->pt_dma_addr); > for (i = 0; i < ppgtt->num_pd_entries; i++) > - __free_page(ppgtt->pt_pages[i]); > - kfree(ppgtt->pt_pages); > + __free_page(ppgtt->pd.page_tables[i].page); > + kfree(ppgtt->pd.page_tables); > } > > static void gen6_ppgtt_cleanup(struct i915_address_space *vm) > @@ -1044,22 +1043,22 @@ alloc: > > static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) > { > + struct i915_page_table_entry *pt; > int i; > > - ppgtt->pt_pages = kcalloc(ppgtt->num_pd_entries, sizeof(struct page *), > - GFP_KERNEL); > - > - if (!ppgtt->pt_pages) > + pt = kcalloc(ppgtt->num_pd_entries, sizeof(*pt), GFP_KERNEL); > + if (!pt) > return -ENOMEM; > > for (i = 0; i < ppgtt->num_pd_entries; i++) { > - ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL); > - if (!ppgtt->pt_pages[i]) { > + pt[i].page = alloc_page(GFP_KERNEL); > + if (!pt->page) { > gen6_ppgtt_free(ppgtt); > return -ENOMEM; > } > } > > + ppgtt->pd.page_tables = pt; > return 0; > } > > @@ -1094,9 +1093,11 @@ static int gen6_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt) > int i; > > for (i = 0; i < ppgtt->num_pd_entries; i++) { > + struct page *page; > dma_addr_t pt_addr; > > - pt_addr = pci_map_page(dev->pdev, ppgtt->pt_pages[i], 0, 4096, > + page = ppgtt->pd.page_tables[i].page; > + pt_addr = pci_map_page(dev->pdev, page, 0, 4096, > PCI_DMA_BIDIRECTIONAL); > > if (pci_dma_mapping_error(dev->pdev, pt_addr)) { > @@ -1140,7 +1141,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) > ppgtt->base.insert_entries = gen6_ppgtt_insert_entries; > ppgtt->base.cleanup = gen6_ppgtt_cleanup; > ppgtt->base.start = 0; > - ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; > + ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; > ppgtt->debug_dump = gen6_dump_ppgtt; > > ppgtt->pd_offset = > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 8f76990..d9bc375 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -187,6 +187,20 @@ struct i915_vma { > u32 flags); > }; > > +struct i915_page_table_entry { > + struct page *page; > +}; > + > +struct i915_page_directory_entry { > + struct page *page; /* NULL for GEN6-GEN7 */ > + struct i915_page_table_entry *page_tables; > +}; > + > +struct i915_page_directory_pointer_entry { > + /* struct page *page; */ > + struct i915_page_directory_entry page_directory[GEN8_LEGACY_PDPES]; > +}; > + > struct i915_address_space { > struct drm_mm mm; > struct drm_device *dev; > @@ -272,11 +286,6 @@ struct i915_hw_ppgtt { > unsigned num_pd_entries; > unsigned num_pd_pages; /* gen8+ */ > union { > - struct page **pt_pages; > - struct page **gen8_pt_pages[GEN8_LEGACY_PDPES]; > - }; > - struct page *pd_pages; > - union { > uint32_t pd_offset; > dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPES]; > }; > @@ -284,6 +293,10 @@ struct i915_hw_ppgtt { > dma_addr_t *pt_dma_addr; > dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPES]; > }; > + union { > + struct i915_page_directory_pointer_entry pdp; > + struct i915_page_directory_entry pd; > + }; > > struct drm_i915_file_private *file_priv; > > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 2/18/2015 11:27 AM, Mika Kuoppala wrote: > Michel Thierry <michel.thierry@intel.com> writes: > >> From: Ben Widawsky <benjamin.widawsky@intel.com> >> >> When we move to dynamic page allocation, keeping page_directory and pagetabs as >> separate structures will help to break actions into simpler tasks. >> >> To help transition the code nicely there is some wasted space in gen6/7. >> This will be ameliorated shortly. >> >> Following the x86 pagetable terminology: >> PDPE = struct i915_page_directory_pointer_entry. >> PDE = struct i915_page_directory_entry [page_directory]. >> PTE = struct i915_page_table_entry [page_tables]. >> >> v2: fixed mismatches after clean-up/rebase. >> >> v3: Clarify the names of the multiple levels of page tables (Daniel) >> >> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2, v3) >> --- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 177 ++++++++++++++++++------------------ >> drivers/gpu/drm/i915/i915_gem_gtt.h | 23 ++++- >> 2 files changed, 107 insertions(+), 93 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index b48b586..98b4698 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -334,7 +334,8 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, >> I915_CACHE_LLC, use_scratch); >> >> while (num_entries) { >> - struct page *page_table = ppgtt->gen8_pt_pages[pdpe][pde]; >> + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; >> + struct page *page_table = pd->page_tables[pde].page; >> >> last_pte = pte + num_entries; >> if (last_pte > GEN8_PTES_PER_PAGE) >> @@ -378,8 +379,12 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, >> if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES)) >> break; >> >> - if (pt_vaddr == NULL) >> - pt_vaddr = kmap_atomic(ppgtt->gen8_pt_pages[pdpe][pde]); >> + if (pt_vaddr == NULL) { >> + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; >> + struct page *page_table = pd->page_tables[pde].page; >> + >> + pt_vaddr = kmap_atomic(page_table); >> + } >> >> pt_vaddr[pte] = >> gen8_pte_encode(sg_page_iter_dma_address(&sg_iter), >> @@ -403,29 +408,33 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, >> } >> } >> >> -static void gen8_free_page_tables(struct page **pt_pages) >> +static void gen8_free_page_tables(struct i915_page_directory_entry *pd) >> { >> int i; >> >> - if (pt_pages == NULL) >> + if (pd->page_tables == NULL) >> return; >> >> for (i = 0; i < GEN8_PDES_PER_PAGE; i++) >> - if (pt_pages[i]) >> - __free_pages(pt_pages[i], 0); >> + if (pd->page_tables[i].page) >> + __free_page(pd->page_tables[i].page); >> } >> >> -static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt) >> +static void gen8_free_page_directories(struct i915_page_directory_entry *pd) > ^ > You only free one directory so why plural here? > >> +{ > If you free the page tables for the directory here.. > >> + kfree(pd->page_tables); >> + __free_page(pd->page); >> +} >> + >> +static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt) >> { >> int i; >> >> for (i = 0; i < ppgtt->num_pd_pages; i++) { >> - gen8_free_page_tables(ppgtt->gen8_pt_pages[i]); >> - kfree(ppgtt->gen8_pt_pages[i]); >> + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); > ...this loop will be cleaner. > > Also consider renaming 'num_pd_pages' to 'num_pd'. But if it does > cause a lot of rebase burden dont worry about it. num_pd_pages will go away in patch #19, so I rather not change that. All other comments addressed in v4. Thanks, -Michel >> + gen8_free_page_directories(&ppgtt->pdp.page_directory[i]); >> kfree(ppgtt->gen8_pt_dma_addr[i]); >> } >> - >> - __free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT)); >> } >> >> static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) >> @@ -460,86 +469,75 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) >> gen8_ppgtt_free(ppgtt); >> } >> >> -static struct page **__gen8_alloc_page_tables(void) >> +static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) >> { >> - struct page **pt_pages; >> int i; >> >> - pt_pages = kcalloc(GEN8_PDES_PER_PAGE, sizeof(struct page *), GFP_KERNEL); >> - if (!pt_pages) >> - return ERR_PTR(-ENOMEM); >> - >> - for (i = 0; i < GEN8_PDES_PER_PAGE; i++) { >> - pt_pages[i] = alloc_page(GFP_KERNEL); >> - if (!pt_pages[i]) >> - goto bail; >> + for (i = 0; i < ppgtt->num_pd_pages; 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]) >> + return -ENOMEM; >> } >> >> - return pt_pages; >> - >> -bail: >> - gen8_free_page_tables(pt_pages); >> - kfree(pt_pages); >> - return ERR_PTR(-ENOMEM); >> + return 0; >> } >> >> -static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt, >> - const int max_pdp) >> +static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) >> { >> - struct page **pt_pages[GEN8_LEGACY_PDPES]; >> - int i, ret; >> + int i, j; >> >> - for (i = 0; i < max_pdp; i++) { >> - pt_pages[i] = __gen8_alloc_page_tables(); >> - if (IS_ERR(pt_pages[i])) { >> - ret = PTR_ERR(pt_pages[i]); >> - goto unwind_out; >> + for (i = 0; i < ppgtt->num_pd_pages; i++) { >> + for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { >> + struct i915_page_table_entry *pt = &ppgtt->pdp.page_directory[i].page_tables[j]; >> + >> + pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO); >> + if (!pt->page) >> + goto unwind_out; >> } >> } >> >> - /* NB: Avoid touching gen8_pt_pages until last to keep the allocation, >> - * "atomic" - for cleanup purposes. >> - */ >> - for (i = 0; i < max_pdp; i++) >> - ppgtt->gen8_pt_pages[i] = pt_pages[i]; >> - >> return 0; >> >> unwind_out: >> - while (i--) { >> - gen8_free_page_tables(pt_pages[i]); >> - kfree(pt_pages[i]); >> - } >> + while (i--) >> + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); >> >> - return ret; >> + return -ENOMEM; >> } >> >> -static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) >> +static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, >> + const int max_pdp) >> { >> int i; >> >> - for (i = 0; i < ppgtt->num_pd_pages; 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]) >> - return -ENOMEM; >> - } >> + for (i = 0; i < max_pdp; i++) { >> + struct i915_page_table_entry *pt; >> >> - return 0; >> -} >> + pt = kcalloc(GEN8_PDES_PER_PAGE, sizeof(*pt), GFP_KERNEL); >> + if (!pt) >> + goto unwind_out; >> >> -static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, >> - const int max_pdp) >> -{ >> - ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT)); >> - if (!ppgtt->pd_pages) >> - return -ENOMEM; >> + ppgtt->pdp.page_directory[i].page = alloc_page(GFP_KERNEL); >> + if (!ppgtt->pdp.page_directory[i].page) >> + goto unwind_out; > If you end up having alloc error here you will leak the previously > allocated pt above. > > Also consider that if you do gen8_ppgtt_allocate_page_directory() and > add null check for pd->page in gen8_free_page_directory you should be able to avoid > the unwinding below completely. > >> + >> + ppgtt->pdp.page_directory[i].page_tables = pt; >> + } >> >> - ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT); >> + ppgtt->num_pd_pages = max_pdp; >> BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPES); >> >> return 0; >> + >> +unwind_out: >> + while (i--) { >> + kfree(ppgtt->pdp.page_directory[i].page_tables); >> + __free_page(ppgtt->pdp.page_directory[i].page); >> + } >> + >> + return -ENOMEM; >> } >> >> static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, >> @@ -551,18 +549,19 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, >> if (ret) >> return ret; >> >> - ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp); >> - if (ret) { >> - __free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT)); >> - return ret; >> - } >> + ret = gen8_ppgtt_allocate_page_tables(ppgtt); >> + if (ret) >> + goto err_out; >> >> ppgtt->num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE; >> >> ret = gen8_ppgtt_allocate_dma(ppgtt); >> - if (ret) >> - gen8_ppgtt_free(ppgtt); >> + if (!ret) >> + return ret; >> >> + /* TODO: Check this for all cases */ > The check for zero return and then returning it with the comment is > confusing. Why not just do the same pattern as in above? > > if (ret) > goto err_out; > > return 0; > > -Mika > >> +err_out: >> + gen8_ppgtt_free(ppgtt); >> return ret; >> } >> >> @@ -573,7 +572,7 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt, >> int ret; >> >> pd_addr = pci_map_page(ppgtt->base.dev->pdev, >> - &ppgtt->pd_pages[pd], 0, >> + ppgtt->pdp.page_directory[pd].page, 0, >> PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); >> >> ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr); >> @@ -593,7 +592,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt, >> struct page *p; >> int ret; >> >> - p = ppgtt->gen8_pt_pages[pd][pt]; >> + p = ppgtt->pdp.page_directory[pd].page_tables[pt].page; >> pt_addr = pci_map_page(ppgtt->base.dev->pdev, >> p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); >> ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr); >> @@ -654,7 +653,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) >> */ >> for (i = 0; i < max_pdp; i++) { >> gen8_ppgtt_pde_t *pd_vaddr; >> - pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]); >> + pd_vaddr = kmap_atomic(ppgtt->pdp.page_directory[i].page); >> for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { >> dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j]; >> pd_vaddr[j] = gen8_pde_encode(ppgtt->base.dev, addr, >> @@ -717,7 +716,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) >> expected); >> seq_printf(m, "\tPDE: %x\n", pd_entry); >> >> - pt_vaddr = kmap_atomic(ppgtt->pt_pages[pde]); >> + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[pde].page); >> for (pte = 0; pte < I915_PPGTT_PT_ENTRIES; pte+=4) { >> unsigned long va = >> (pde * PAGE_SIZE * I915_PPGTT_PT_ENTRIES) + >> @@ -922,7 +921,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm, >> if (last_pte > I915_PPGTT_PT_ENTRIES) >> last_pte = I915_PPGTT_PT_ENTRIES; >> >> - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); >> + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); >> >> for (i = first_pte; i < last_pte; i++) >> pt_vaddr[i] = scratch_pte; >> @@ -951,7 +950,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm, >> pt_vaddr = NULL; >> for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) { >> if (pt_vaddr == NULL) >> - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); >> + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); >> >> pt_vaddr[act_pte] = >> vm->pte_encode(sg_page_iter_dma_address(&sg_iter), >> @@ -986,8 +985,8 @@ static void gen6_ppgtt_free(struct i915_hw_ppgtt *ppgtt) >> >> kfree(ppgtt->pt_dma_addr); >> for (i = 0; i < ppgtt->num_pd_entries; i++) >> - __free_page(ppgtt->pt_pages[i]); >> - kfree(ppgtt->pt_pages); >> + __free_page(ppgtt->pd.page_tables[i].page); >> + kfree(ppgtt->pd.page_tables); >> } >> >> static void gen6_ppgtt_cleanup(struct i915_address_space *vm) >> @@ -1044,22 +1043,22 @@ alloc: >> >> static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) >> { >> + struct i915_page_table_entry *pt; >> int i; >> >> - ppgtt->pt_pages = kcalloc(ppgtt->num_pd_entries, sizeof(struct page *), >> - GFP_KERNEL); >> - >> - if (!ppgtt->pt_pages) >> + pt = kcalloc(ppgtt->num_pd_entries, sizeof(*pt), GFP_KERNEL); >> + if (!pt) >> return -ENOMEM; >> >> for (i = 0; i < ppgtt->num_pd_entries; i++) { >> - ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL); >> - if (!ppgtt->pt_pages[i]) { >> + pt[i].page = alloc_page(GFP_KERNEL); >> + if (!pt->page) { >> gen6_ppgtt_free(ppgtt); >> return -ENOMEM; >> } >> } >> >> + ppgtt->pd.page_tables = pt; >> return 0; >> } >> >> @@ -1094,9 +1093,11 @@ static int gen6_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt) >> int i; >> >> for (i = 0; i < ppgtt->num_pd_entries; i++) { >> + struct page *page; >> dma_addr_t pt_addr; >> >> - pt_addr = pci_map_page(dev->pdev, ppgtt->pt_pages[i], 0, 4096, >> + page = ppgtt->pd.page_tables[i].page; >> + pt_addr = pci_map_page(dev->pdev, page, 0, 4096, >> PCI_DMA_BIDIRECTIONAL); >> >> if (pci_dma_mapping_error(dev->pdev, pt_addr)) { >> @@ -1140,7 +1141,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) >> ppgtt->base.insert_entries = gen6_ppgtt_insert_entries; >> ppgtt->base.cleanup = gen6_ppgtt_cleanup; >> ppgtt->base.start = 0; >> - ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; >> + ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; >> ppgtt->debug_dump = gen6_dump_ppgtt; >> >> ppgtt->pd_offset = >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h >> index 8f76990..d9bc375 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h >> @@ -187,6 +187,20 @@ struct i915_vma { >> u32 flags); >> }; >> >> +struct i915_page_table_entry { >> + struct page *page; >> +}; >> + >> +struct i915_page_directory_entry { >> + struct page *page; /* NULL for GEN6-GEN7 */ >> + struct i915_page_table_entry *page_tables; >> +}; >> + >> +struct i915_page_directory_pointer_entry { >> + /* struct page *page; */ >> + struct i915_page_directory_entry page_directory[GEN8_LEGACY_PDPES]; >> +}; >> + >> struct i915_address_space { >> struct drm_mm mm; >> struct drm_device *dev; >> @@ -272,11 +286,6 @@ struct i915_hw_ppgtt { >> unsigned num_pd_entries; >> unsigned num_pd_pages; /* gen8+ */ >> union { >> - struct page **pt_pages; >> - struct page **gen8_pt_pages[GEN8_LEGACY_PDPES]; >> - }; >> - struct page *pd_pages; >> - union { >> uint32_t pd_offset; >> dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPES]; >> }; >> @@ -284,6 +293,10 @@ struct i915_hw_ppgtt { >> dma_addr_t *pt_dma_addr; >> dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPES]; >> }; >> + union { >> + struct i915_page_directory_pointer_entry pdp; >> + struct i915_page_directory_entry pd; >> + }; >> >> struct drm_i915_file_private *file_priv; >> >> -- >> 2.1.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b48b586..98b4698 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -334,7 +334,8 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, I915_CACHE_LLC, use_scratch); while (num_entries) { - struct page *page_table = ppgtt->gen8_pt_pages[pdpe][pde]; + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; + struct page *page_table = pd->page_tables[pde].page; last_pte = pte + num_entries; if (last_pte > GEN8_PTES_PER_PAGE) @@ -378,8 +379,12 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, if (WARN_ON(pdpe >= GEN8_LEGACY_PDPES)) break; - if (pt_vaddr == NULL) - pt_vaddr = kmap_atomic(ppgtt->gen8_pt_pages[pdpe][pde]); + if (pt_vaddr == NULL) { + struct i915_page_directory_entry *pd = &ppgtt->pdp.page_directory[pdpe]; + struct page *page_table = pd->page_tables[pde].page; + + pt_vaddr = kmap_atomic(page_table); + } pt_vaddr[pte] = gen8_pte_encode(sg_page_iter_dma_address(&sg_iter), @@ -403,29 +408,33 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, } } -static void gen8_free_page_tables(struct page **pt_pages) +static void gen8_free_page_tables(struct i915_page_directory_entry *pd) { int i; - if (pt_pages == NULL) + if (pd->page_tables == NULL) return; for (i = 0; i < GEN8_PDES_PER_PAGE; i++) - if (pt_pages[i]) - __free_pages(pt_pages[i], 0); + if (pd->page_tables[i].page) + __free_page(pd->page_tables[i].page); } -static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt) +static void gen8_free_page_directories(struct i915_page_directory_entry *pd) +{ + kfree(pd->page_tables); + __free_page(pd->page); +} + +static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt) { int i; for (i = 0; i < ppgtt->num_pd_pages; i++) { - gen8_free_page_tables(ppgtt->gen8_pt_pages[i]); - kfree(ppgtt->gen8_pt_pages[i]); + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); + gen8_free_page_directories(&ppgtt->pdp.page_directory[i]); kfree(ppgtt->gen8_pt_dma_addr[i]); } - - __free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT)); } static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) @@ -460,86 +469,75 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) gen8_ppgtt_free(ppgtt); } -static struct page **__gen8_alloc_page_tables(void) +static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) { - struct page **pt_pages; int i; - pt_pages = kcalloc(GEN8_PDES_PER_PAGE, sizeof(struct page *), GFP_KERNEL); - if (!pt_pages) - return ERR_PTR(-ENOMEM); - - for (i = 0; i < GEN8_PDES_PER_PAGE; i++) { - pt_pages[i] = alloc_page(GFP_KERNEL); - if (!pt_pages[i]) - goto bail; + for (i = 0; i < ppgtt->num_pd_pages; 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]) + return -ENOMEM; } - return pt_pages; - -bail: - gen8_free_page_tables(pt_pages); - kfree(pt_pages); - return ERR_PTR(-ENOMEM); + return 0; } -static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt, - const int max_pdp) +static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) { - struct page **pt_pages[GEN8_LEGACY_PDPES]; - int i, ret; + int i, j; - for (i = 0; i < max_pdp; i++) { - pt_pages[i] = __gen8_alloc_page_tables(); - if (IS_ERR(pt_pages[i])) { - ret = PTR_ERR(pt_pages[i]); - goto unwind_out; + for (i = 0; i < ppgtt->num_pd_pages; i++) { + for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { + struct i915_page_table_entry *pt = &ppgtt->pdp.page_directory[i].page_tables[j]; + + pt->page = alloc_page(GFP_KERNEL | __GFP_ZERO); + if (!pt->page) + goto unwind_out; } } - /* NB: Avoid touching gen8_pt_pages until last to keep the allocation, - * "atomic" - for cleanup purposes. - */ - for (i = 0; i < max_pdp; i++) - ppgtt->gen8_pt_pages[i] = pt_pages[i]; - return 0; unwind_out: - while (i--) { - gen8_free_page_tables(pt_pages[i]); - kfree(pt_pages[i]); - } + while (i--) + gen8_free_page_tables(&ppgtt->pdp.page_directory[i]); - return ret; + return -ENOMEM; } -static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt) +static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, + const int max_pdp) { int i; - for (i = 0; i < ppgtt->num_pd_pages; 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]) - return -ENOMEM; - } + for (i = 0; i < max_pdp; i++) { + struct i915_page_table_entry *pt; - return 0; -} + pt = kcalloc(GEN8_PDES_PER_PAGE, sizeof(*pt), GFP_KERNEL); + if (!pt) + goto unwind_out; -static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt, - const int max_pdp) -{ - ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT)); - if (!ppgtt->pd_pages) - return -ENOMEM; + ppgtt->pdp.page_directory[i].page = alloc_page(GFP_KERNEL); + if (!ppgtt->pdp.page_directory[i].page) + goto unwind_out; + + ppgtt->pdp.page_directory[i].page_tables = pt; + } - ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT); + ppgtt->num_pd_pages = max_pdp; BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPES); return 0; + +unwind_out: + while (i--) { + kfree(ppgtt->pdp.page_directory[i].page_tables); + __free_page(ppgtt->pdp.page_directory[i].page); + } + + return -ENOMEM; } static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, @@ -551,18 +549,19 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt, if (ret) return ret; - ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp); - if (ret) { - __free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT)); - return ret; - } + ret = gen8_ppgtt_allocate_page_tables(ppgtt); + if (ret) + goto err_out; ppgtt->num_pd_entries = max_pdp * GEN8_PDES_PER_PAGE; ret = gen8_ppgtt_allocate_dma(ppgtt); - if (ret) - gen8_ppgtt_free(ppgtt); + if (!ret) + return ret; + /* TODO: Check this for all cases */ +err_out: + gen8_ppgtt_free(ppgtt); return ret; } @@ -573,7 +572,7 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt, int ret; pd_addr = pci_map_page(ppgtt->base.dev->pdev, - &ppgtt->pd_pages[pd], 0, + ppgtt->pdp.page_directory[pd].page, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr); @@ -593,7 +592,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt, struct page *p; int ret; - p = ppgtt->gen8_pt_pages[pd][pt]; + p = ppgtt->pdp.page_directory[pd].page_tables[pt].page; pt_addr = pci_map_page(ppgtt->base.dev->pdev, p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr); @@ -654,7 +653,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) */ for (i = 0; i < max_pdp; i++) { gen8_ppgtt_pde_t *pd_vaddr; - pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]); + pd_vaddr = kmap_atomic(ppgtt->pdp.page_directory[i].page); for (j = 0; j < GEN8_PDES_PER_PAGE; j++) { dma_addr_t addr = ppgtt->gen8_pt_dma_addr[i][j]; pd_vaddr[j] = gen8_pde_encode(ppgtt->base.dev, addr, @@ -717,7 +716,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) expected); seq_printf(m, "\tPDE: %x\n", pd_entry); - pt_vaddr = kmap_atomic(ppgtt->pt_pages[pde]); + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[pde].page); for (pte = 0; pte < I915_PPGTT_PT_ENTRIES; pte+=4) { unsigned long va = (pde * PAGE_SIZE * I915_PPGTT_PT_ENTRIES) + @@ -922,7 +921,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm, if (last_pte > I915_PPGTT_PT_ENTRIES) last_pte = I915_PPGTT_PT_ENTRIES; - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); for (i = first_pte; i < last_pte; i++) pt_vaddr[i] = scratch_pte; @@ -951,7 +950,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm, pt_vaddr = NULL; for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) { if (pt_vaddr == NULL) - pt_vaddr = kmap_atomic(ppgtt->pt_pages[act_pt]); + pt_vaddr = kmap_atomic(ppgtt->pd.page_tables[act_pt].page); pt_vaddr[act_pte] = vm->pte_encode(sg_page_iter_dma_address(&sg_iter), @@ -986,8 +985,8 @@ static void gen6_ppgtt_free(struct i915_hw_ppgtt *ppgtt) kfree(ppgtt->pt_dma_addr); for (i = 0; i < ppgtt->num_pd_entries; i++) - __free_page(ppgtt->pt_pages[i]); - kfree(ppgtt->pt_pages); + __free_page(ppgtt->pd.page_tables[i].page); + kfree(ppgtt->pd.page_tables); } static void gen6_ppgtt_cleanup(struct i915_address_space *vm) @@ -1044,22 +1043,22 @@ alloc: static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt) { + struct i915_page_table_entry *pt; int i; - ppgtt->pt_pages = kcalloc(ppgtt->num_pd_entries, sizeof(struct page *), - GFP_KERNEL); - - if (!ppgtt->pt_pages) + pt = kcalloc(ppgtt->num_pd_entries, sizeof(*pt), GFP_KERNEL); + if (!pt) return -ENOMEM; for (i = 0; i < ppgtt->num_pd_entries; i++) { - ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL); - if (!ppgtt->pt_pages[i]) { + pt[i].page = alloc_page(GFP_KERNEL); + if (!pt->page) { gen6_ppgtt_free(ppgtt); return -ENOMEM; } } + ppgtt->pd.page_tables = pt; return 0; } @@ -1094,9 +1093,11 @@ static int gen6_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt) int i; for (i = 0; i < ppgtt->num_pd_entries; i++) { + struct page *page; dma_addr_t pt_addr; - pt_addr = pci_map_page(dev->pdev, ppgtt->pt_pages[i], 0, 4096, + page = ppgtt->pd.page_tables[i].page; + pt_addr = pci_map_page(dev->pdev, page, 0, 4096, PCI_DMA_BIDIRECTIONAL); if (pci_dma_mapping_error(dev->pdev, pt_addr)) { @@ -1140,7 +1141,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) ppgtt->base.insert_entries = gen6_ppgtt_insert_entries; ppgtt->base.cleanup = gen6_ppgtt_cleanup; ppgtt->base.start = 0; - ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; + ppgtt->base.total = ppgtt->num_pd_entries * I915_PPGTT_PT_ENTRIES * PAGE_SIZE; ppgtt->debug_dump = gen6_dump_ppgtt; ppgtt->pd_offset = diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 8f76990..d9bc375 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -187,6 +187,20 @@ struct i915_vma { u32 flags); }; +struct i915_page_table_entry { + struct page *page; +}; + +struct i915_page_directory_entry { + struct page *page; /* NULL for GEN6-GEN7 */ + struct i915_page_table_entry *page_tables; +}; + +struct i915_page_directory_pointer_entry { + /* struct page *page; */ + struct i915_page_directory_entry page_directory[GEN8_LEGACY_PDPES]; +}; + struct i915_address_space { struct drm_mm mm; struct drm_device *dev; @@ -272,11 +286,6 @@ struct i915_hw_ppgtt { unsigned num_pd_entries; unsigned num_pd_pages; /* gen8+ */ union { - struct page **pt_pages; - struct page **gen8_pt_pages[GEN8_LEGACY_PDPES]; - }; - struct page *pd_pages; - union { uint32_t pd_offset; dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPES]; }; @@ -284,6 +293,10 @@ struct i915_hw_ppgtt { dma_addr_t *pt_dma_addr; dma_addr_t *gen8_pt_dma_addr[GEN8_LEGACY_PDPES]; }; + union { + struct i915_page_directory_pointer_entry pdp; + struct i915_page_directory_entry pd; + }; struct drm_i915_file_private *file_priv;