[Intel-gfx] [PATCH] drm/i915: Introduce a new create ioctl for user specified placement

[Daniel Vetter]

On Tue, Jul 09, 2013 at 09:46:58AM +0100, Chris Wilson wrote:
> On Mon, Jul 08, 2013 at 10:57:09PM +0200, Daniel Vetter wrote:
> > On Fri, Jul 05, 2013 at 02:42:04PM +0100, Chris Wilson wrote:
> > > Despite being a unified memory architecture (UMA) some bits of memory
> > > are more equal than others. In particular we have the thorny issue of
> > > stolen memory, memory stolen from the system by the BIOS and reserved
> > > for igfx use. Stolen memory is required for some functions of the GPU
> > > and display engine, but in general it goes wasted. Whilst we cannot
> > > return it back to the system, we need to find some other method for
> > > utilising it. As we do not support direct access to the physical address
> > > in the stolen region, it behaves like a different class of memory,
> > > closer in kin to local GPU memory. This strongly suggests that we need a
> > > placement model like TTM if we are to fully utilize these discrete
> > > chunks of differing memory.
> > > 
> > > This new create ioctl therefore exists to allow the user to create these
> > > second class buffer objects from stolen memory. At the moment direct
> > > access by the CPU through mmaps and pread/pwrite are verboten on the
> > > objects, and so the user must be aware of the limitations of the objects
> > > created. Yet, those limitations rarely reduce the desired functionality
> > > in many use cases and so the user should be able to easily fill the
> > > stolen memory and so help to reduce overall memory pressure.
> > > 
> > > The most obvious use case for stolen memory is for the creation of objects
> > > for the display engine which already have very similar restrictions on
> > > access. However, we want a reasonably general ioctl in order to cater
> > > for diverse scenarios beyond the author's imagination.
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > Looks like a sane idea. But I think we should call the new placement flags
> > placaments, not domains, since imo they are a rather different concept
> > than what we currently call domains.
> > 
> > I also wonder whether we should have some for of initial domain (for
> > people who want to do the clflushing up-front, or in case we ever get a
> > clflushed page-cache), but the spare bits in flags should be good enough.
> > Or maybe leave the domain ioctl struct member and move the placement
> > defines to the high bits.
> 
> placement, read_domains, write_domain?

I think just domains is good enough since we've essentially ditched the
disdinction, at least for cache coherency. And at creation time there's no
need to sync to any writers.

> FLAG_UNINITIALIZED && capable(SYS_ADMIN) - avoid having to clear stolen?

I've thought more of FLAG_PREALLOC to force the kernel to grab the backing
storage and put the object onto the unbound list. With the clflushing by
setting the object to the GTT domain (we'd need to adjust our
set_to_gtt_domain function to allow that on unbound objects)
userspace could then chose to do expensive setup costs up-front.

I don't think we should be in the business of handing uncleared memory to
userspace, even for root.
 
> domains could be an u16 and still allow for expansion,
> placement and tiling could be as small as a u8. But is it worth triming
> the bit count? We have to keep the struct below 128 to benefit from the
> onstack copy, and keeping the copy as small as possible is good, but we
> should also leave room for future expansion. So include pad[1234]?

Atm the only thing I see coming up is softpin support for ppgtt, that'd
need another u64 gtt_offset (or maybe u64 reserved; /* gtt offset for
softpining */ for now). I don't see any other need where we'd need more
flags.

> I don't see this as being a (comparatively) frequently called function.
> 
> > And I want i-g-t tests, both basic functionality tests and checking
> > whether all the verboten stuff is indeed forbidden.
> 
> We don't have enough queries for userspace to check that the kernel is
> obeying its instructions. But we can at least check the restrictions on
> ABI are enforced.

Yeah, I was only thinking of the basic abi stuff like whether mmap,
pwrite, pread, ... really doesn't work.

One ugly part is that conceptually I think we want to support dma_buf
exporting of stolen memory objects. After all exporting scanout buffers
for rendering by the dgpu is the prime use-case of dma_buf.

But atm all the importers in drm (us somewhat included) presume that the
sg list passed in is backed by real struct pages. Fixing that up on our
side is just a bit of grunt work (writing kmap and mmap support on the
expoert and writing a special pwrite/pread code and mmap code for imported
dma_bufs). But for ttm that requires us to make dma_bufs first-class
citizens in ttm with their own kmap and mmap functions (the required
abstraction is already there in ttm). That isn't really something I want
to sign up for right now.

There's also the pesky issue of lack of sensible test infrastructure, ime
even the basic nouveau tests we have in igt simply blow up way too often.

Disallowing dma_buf export isn't really an option, since that'll break
wayland and dri3 (if dri3 ever ships). Denying just the attach operation otoh
will break prime but keep wayland/dri3 self-imports working.

Atm I don't have a good idea how to solve this conundrum :(

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch