Michel Thierry <michel.thierry@xxxxxxxxx> writes: > On 5/22/2015 6:05 PM, Mika Kuoppala wrote: >> Lay out scratch page structure in similar manner than other >> paging structures. This allows us to use the same tools for >> setup and teardown. >> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 89 ++++++++++++++++++++----------------- >> drivers/gpu/drm/i915/i915_gem_gtt.h | 9 ++-- >> 2 files changed, 54 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index 4f9a000..43fa543 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -301,11 +301,12 @@ static gen6_pte_t iris_pte_encode(dma_addr_t addr, >> return pte; >> } >> >> -static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p) >> +static int __setup_page_dma(struct drm_device *dev, >> + struct i915_page_dma *p, gfp_t flags) >> { >> struct device *device = &dev->pdev->dev; >> >> - p->page = alloc_page(GFP_KERNEL); >> + p->page = alloc_page(flags); >> if (!p->page) >> return -ENOMEM; >> >> @@ -320,6 +321,11 @@ static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p) >> return 0; >> } >> >> +static int setup_page_dma(struct drm_device *dev, struct i915_page_dma *p) >> +{ >> + return __setup_page_dma(dev, p, GFP_KERNEL); >> +} >> + >> static void cleanup_page_dma(struct drm_device *dev, struct i915_page_dma *p) >> { >> if (WARN_ON(!p->page)) >> @@ -388,7 +394,8 @@ static void gen8_initialize_pt(struct i915_address_space *vm, >> { >> gen8_pte_t scratch_pte; >> >> - scratch_pte = gen8_pte_encode(vm->scratch.addr, I915_CACHE_LLC, true); >> + scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page), >> + I915_CACHE_LLC, true); >> >> fill_px(vm->dev, pt, scratch_pte); >> } >> @@ -515,7 +522,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, >> unsigned num_entries = length >> PAGE_SHIFT; >> unsigned last_pte, i; >> >> - scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr, >> + scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page), >> I915_CACHE_LLC, use_scratch); >> >> while (num_entries) { >> @@ -1021,7 +1028,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) >> uint32_t pte, pde, temp; >> uint32_t start = ppgtt->base.start, length = ppgtt->base.total; >> >> - scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0); >> + scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true, 0); >> >> gen6_for_each_pde(unused, &ppgtt->pd, start, length, temp, pde) { >> u32 expected; >> @@ -1256,7 +1263,8 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm, >> unsigned first_pte = first_entry % GEN6_PTES; >> unsigned last_pte, i; >> >> - scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0); >> + scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), >> + I915_CACHE_LLC, true, 0); >> >> while (num_entries) { >> last_pte = first_pte + num_entries; >> @@ -1314,9 +1322,10 @@ static void gen6_initialize_pt(struct i915_address_space *vm, >> { >> gen6_pte_t scratch_pte; >> >> - WARN_ON(vm->scratch.addr == 0); >> + WARN_ON(px_dma(vm->scratch_page) == 0); >> >> - scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true, 0); >> + scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), >> + I915_CACHE_LLC, true, 0); >> >> fill32_px(vm->dev, pt, scratch_pte); >> } >> @@ -1553,13 +1562,14 @@ static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> ppgtt->base.dev = dev; >> - ppgtt->base.scratch = dev_priv->gtt.base.scratch; >> + ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page; >> >> if (INTEL_INFO(dev)->gen < 8) >> return gen6_ppgtt_init(ppgtt); >> else >> return gen8_ppgtt_init(ppgtt); >> } >> + >> int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> @@ -1874,7 +1884,7 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm, >> first_entry, num_entries, max_entries)) >> num_entries = max_entries; >> >> - scratch_pte = gen8_pte_encode(vm->scratch.addr, >> + scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page), >> I915_CACHE_LLC, >> use_scratch); >> for (i = 0; i < num_entries; i++) >> @@ -1900,7 +1910,8 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm, >> first_entry, num_entries, max_entries)) >> num_entries = max_entries; >> >> - scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, use_scratch, 0); >> + scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), >> + I915_CACHE_LLC, use_scratch, 0); >> >> for (i = 0; i < num_entries; i++) >> iowrite32(scratch_pte, >t_base[i]); >> @@ -2157,42 +2168,40 @@ void i915_global_gtt_cleanup(struct drm_device *dev) >> vm->cleanup(vm); >> } >> >> -static int setup_scratch_page(struct drm_device *dev) >> +static int alloc_scratch_page(struct i915_address_space *vm) >> { >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct page *page; >> - dma_addr_t dma_addr; >> + struct i915_page_scratch *sp; >> + int ret; >> + >> + WARN_ON(vm->scratch_page); >> >> - page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO); >> - if (page == NULL) >> + sp = kzalloc(sizeof(*sp), GFP_KERNEL); >> + if (sp == NULL) >> return -ENOMEM; >> - set_pages_uc(page, 1); >> >> -#ifdef CONFIG_INTEL_IOMMU >> - dma_addr = pci_map_page(dev->pdev, page, 0, PAGE_SIZE, >> - PCI_DMA_BIDIRECTIONAL); >> - if (pci_dma_mapping_error(dev->pdev, dma_addr)) { >> - __free_page(page); >> - return -EINVAL; >> + ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO); >> + if (ret) { >> + kfree(sp); >> + return ret; >> } >> -#else >> - dma_addr = page_to_phys(page); >> -#endif > > Should we keep a no-iommu option? > This seems to have been added for gen6 (gtt). > The dma_map_page should do the right thing with and without iommu. I really don't understand why we would need the no-iommu option. If there is no iommu, we get nommu_map_page() which is effectively page_to_phys(). -Mika >> - dev_priv->gtt.base.scratch.page = page; >> - dev_priv->gtt.base.scratch.addr = dma_addr; >> + >> + set_pages_uc(px_page(sp), 1); >> + >> + vm->scratch_page = sp; >> >> return 0; >> } >> >> -static void teardown_scratch_page(struct drm_device *dev) >> +static void free_scratch_page(struct i915_address_space *vm) >> { >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct page *page = dev_priv->gtt.base.scratch.page; >> + struct i915_page_scratch *sp = vm->scratch_page; >> >> - set_pages_wb(page, 1); >> - pci_unmap_page(dev->pdev, dev_priv->gtt.base.scratch.addr, >> - PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); >> - __free_page(page); >> + set_pages_wb(px_page(sp), 1); >> + >> + cleanup_px(vm->dev, sp); >> + kfree(sp); >> + >> + vm->scratch_page = NULL; >> } >> >> static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl) >> @@ -2300,7 +2309,7 @@ static int ggtt_probe_common(struct drm_device *dev, >> return -ENOMEM; >> } >> >> - ret = setup_scratch_page(dev); >> + ret = alloc_scratch_page(&dev_priv->gtt.base); >> if (ret) { >> DRM_ERROR("Scratch setup failed\n"); >> /* iounmap will also get called at remove, but meh */ >> @@ -2479,7 +2488,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm) >> struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base); >> >> iounmap(gtt->gsm); >> - teardown_scratch_page(vm->dev); >> + free_scratch_page(vm); >> } >> >> static int i915_gmch_probe(struct drm_device *dev, >> @@ -2543,13 +2552,13 @@ int i915_gem_gtt_init(struct drm_device *dev) >> dev_priv->gtt.base.cleanup = gen6_gmch_remove; >> } >> >> + gtt->base.dev = dev; >> + >> ret = gtt->gtt_probe(dev, >t->base.total, >t->stolen_size, >> >t->mappable_base, >t->mappable_end); >> if (ret) >> return ret; >> >> - gtt->base.dev = dev; >> - >> /* GMADR is the PCI mmio aperture into the global GTT. */ >> DRM_INFO("Memory usable by graphics device = %lluM\n", >> gtt->base.total >> 20); >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h >> index 006b839..1fd4041 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h >> @@ -217,6 +217,10 @@ struct i915_page_dma { >> #define px_page(px) (px_base(px)->page) >> #define px_dma(px) (px_base(px)->daddr) >> >> +struct i915_page_scratch { >> + struct i915_page_dma base; >> +}; >> + >> struct i915_page_table { >> struct i915_page_dma base; >> >> @@ -243,10 +247,7 @@ struct i915_address_space { >> u64 start; /* Start offset always 0 for dri2 */ >> u64 total; /* size addr space maps (ex. 2GB for ggtt) */ >> >> - struct { >> - dma_addr_t addr; >> - struct page *page; >> - } scratch; >> + struct i915_page_scratch *scratch_page; >> >> /** >> * List of objects currently involved in rendering. >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx