On ke, 2017-07-26 at 14:25 +0100, Chris Wilson wrote:
> We use WC pages for coherent writes into the ppGTT on !llc
> architectuures. However, to create a WC page requires a stop_machine(),
> i.e. is very slow. To compensate we currently keep a per-vm cache of
> recently freed pages, but we still see the slow startup of new contexts.
> We can amoritize that cost slightly by allocating WC pages in small
> batches (PAGEVEC_SIZE == 14) and since creating a WC page implies a
> stop_machine() there is no penalty for keeping that stash global.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
<SNIP>
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1464,6 +1464,9 @@ struct i915_gem_mm {
> struct llist_head free_list;
> struct work_struct free_work;
>
> + /** Small stash of WC pages */
Please briefly add the "why".
> + struct pagevec wc_stash;
> +
> /** Usable portion of the GTT for GEM */
> dma_addr_t stolen_base; /* limited to low memory (32-bit) */
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 10aa7762d9a6..ad4e48435853 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -351,39 +351,85 @@ static gen6_pte_t iris_pte_encode(dma_addr_t addr,
>
> static struct page *vm_alloc_page(struct i915_address_space *vm, gfp_t gfp)
> {
> - struct page *page;
> + struct pagevec *pvec = &vm->free_pages;
>
> if (I915_SELFTEST_ONLY(should_fail(&vm->fault_attr, 1)))
> i915_gem_shrink_all(vm->i915);
>
> - if (vm->free_pages.nr)
> - return vm->free_pages.pages[--vm->free_pages.nr];
> + if (likely(pvec->nr))
> + return pvec->pages[--pvec->nr];
> +
> + if (!vm->pt_kmap_wc)
> + return alloc_page(gfp);
> +
> + lockdep_assert_held(&vm->i915->drm.struct_mutex);
Lift this to the beginning of the func, should trigger easier.
> -static void vm_free_pages_release(struct i915_address_space *vm)
> +static void vm_free_pages_release(struct i915_address_space *vm,
> + bool immediate)
> {
> - GEM_BUG_ON(!pagevec_count(&vm->free_pages));
> + struct pagevec *pvec = &vm->free_pages;
> +
> + GEM_BUG_ON(!pagevec_count(pvec));
> +
> + if (vm->pt_kmap_wc) {
> + struct pagevec *stash = &vm->i915->mm.wc_stash;
>
> - if (vm->pt_kmap_wc)
> - set_pages_array_wb(vm->free_pages.pages,
> - pagevec_count(&vm->free_pages));
> + /* When we use WC, first fill up the global stash and then
> + * only if full immediately free the overflow.
> + */
> +
> + lockdep_assert_held(&vm->i915->drm.struct_mutex);
I'd again lift this up to the beginning.
> @@ -447,12 +493,31 @@ static void fill_page_dma_32(struct i915_address_space
> *vm,
> static int
> setup_scratch_page(struct i915_address_space *vm, gfp_t gfp)
> {
> - return __setup_page_dma(vm, &vm->scratch_page, gfp | __GFP_ZERO);
> + struct page *page;
> + dma_addr_t addr;
> +
> + page = alloc_page(gfp | __GFP_ZERO);
> + if (unlikely(!page))
> + return -ENOMEM;
> +
> + addr = dma_map_page(vm->dma, page, 0, PAGE_SIZE,
> + PCI_DMA_BIDIRECTIONAL);
> + if (unlikely(dma_mapping_error(vm->dma, addr))) {
> + __free_page(page);
> + return -ENOMEM;
> + }
> +
> + vm->scratch_page.page = page;
> + vm->scratch_page.daddr = addr;
> + return 0;
Unrelated hunk, please split.
> }
>
> static void cleanup_scratch_page(struct i915_address_space *vm)
> {
> - cleanup_page_dma(vm, &vm->scratch_page);
> + struct i915_page_dma *p = &vm->scratch_page;
> +
> + dma_unmap_page(vm->dma, p->daddr, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
> + __free_page(p->page);
Ditto.
> }
>
> static struct i915_page_table *alloc_pt(struct i915_address_space *vm)
> @@ -1332,18 +1397,18 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> *ppgtt)
> 1ULL << 48 :
> 1ULL << 32;
>
> - ret = gen8_init_scratch(&ppgtt->base);
> - if (ret) {
> - ppgtt->base.total = 0;
> - return ret;
> - }
> -
> /* There are only few exceptions for gen >=6. chv and bxt.
> * And we are not sure about the latter so play safe for now.
> */
> if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv))
> ppgtt->base.pt_kmap_wc = true;
>
> + ret = gen8_init_scratch(&ppgtt->base);
> + if (ret) {
> + ppgtt->base.total = 0;
> + return ret;
> + }
> +
More unrelated hunks.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
