Quoting Joonas Lahtinen (2017-08-11 08:34:02)
> On ke, 2017-07-26 at 14:25 +0100, Chris Wilson wrote:
> > - 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.
I placed it here to show that only this path required the extra lock
with a plan that we would take a more specialised lock at this point.
>
> > @@ -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.
Ah, but they weren't.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
