[Intel-gfx] [PATCH 01/16] drm/i915: Keep a small stash of preallocated WC pages

[Chris Wilson]

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