On Mon, 6 Jul 2020 at 07:19, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
>
> The GEM object is grossly overweight for the practicality of tracking
> large numbers of individual pages, yet it is currently our only
> abstraction for tracking DMA allocations. Since those allocations need
> to be reserved upfront before an operation, and that we need to break
> away from simple system memory, we need to ditch using plain struct page
> wrappers.
>
> In the process, we drop the WC mapping as we ended up clflushing
> everything anyway due to various issues across a wider range of
> platforms. Though in a future step, we need to drop the kmap_atomic
> approach which suggests we need to pre-map all the pages and keep them
> mapped.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
<snip>
>
> -int setup_scratch_page(struct i915_address_space *vm, gfp_t gfp)
> +int setup_scratch_page(struct i915_address_space *vm)
> {
> unsigned long size;
>
> @@ -338,21 +174,22 @@ int setup_scratch_page(struct i915_address_space *vm, gfp_t gfp)
> */
> size = I915_GTT_PAGE_SIZE_4K;
> if (i915_vm_is_4lvl(vm) &&
> - HAS_PAGE_SIZES(vm->i915, I915_GTT_PAGE_SIZE_64K)) {
> + HAS_PAGE_SIZES(vm->i915, I915_GTT_PAGE_SIZE_64K))
> size = I915_GTT_PAGE_SIZE_64K;
> - gfp |= __GFP_NOWARN;
> - }
> - gfp |= __GFP_ZERO | __GFP_RETRY_MAYFAIL;
>
> do {
> - unsigned int order = get_order(size);
> - struct page *page;
> - dma_addr_t addr;
> + struct drm_i915_gem_object *obj;
>
> - page = alloc_pages(gfp, order);
> - if (unlikely(!page))
> + obj = vm->alloc_pt_dma(vm, size);
> + if (IS_ERR(obj))
> goto skip;
>
> + if (pin_pt_dma(vm, obj))
> + goto skip_obj;
> +
> + if (obj->mm.page_sizes.sg < size)
> + goto skip_obj;
> +
We should still check the alignment of the final dma address
somewhere, in the case of 64K. I have for sure seen dma misalignment
here before.
> /*
> * Use a non-zero scratch page for debugging.
> *
> @@ -362,61 +199,28 @@ int setup_scratch_page(struct i915_address_space *vm, gfp_t gfp)
> * should it ever be accidentally used, the effect should be
> * fairly benign.
> */
> - poison_scratch_page(page, size);
> -
> - addr = dma_map_page_attrs(vm->dma,
> - page, 0, size,
> - PCI_DMA_BIDIRECTIONAL,
> - DMA_ATTR_SKIP_CPU_SYNC |
> - DMA_ATTR_NO_WARN);
> - if (unlikely(dma_mapping_error(vm->dma, addr)))
> - goto free_page;
> -
> - if (unlikely(!IS_ALIGNED(addr, size)))
> - goto unmap_page;
> -
> - vm->scratch[0].base.page = page;
> - vm->scratch[0].base.daddr = addr;
> - vm->scratch_order = order;
> + poison_scratch_page(obj);
Since this is now an internal object, which lacks proper clearing, do
we need to nuke the page(s) somewhere, since it is visible to
userspace? The posion_scratch seems to only be for debug builds.
> +
> + vm->scratch[0] = obj;
> + vm->scratch_order = get_order(size);
> return 0;
>
> -unmap_page:
> - dma_unmap_page(vm->dma, addr, size, PCI_DMA_BIDIRECTIONAL);
> -free_page:
> - __free_pages(page, order);
> +skip_obj:
> + i915_gem_object_put(obj);
> skip:
> if (size == I915_GTT_PAGE_SIZE_4K)
> return -ENOMEM;
>
> size = I915_GTT_PAGE_SIZE_4K;
> - gfp &= ~__GFP_NOWARN;
> } while (1);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
