[Intel-gfx] [PATCH v3 07/15] drm/i915: Add functions to allocate / release gem obj for GuC

[Chris Wilson]

On Tue, Apr 21, 2015 at 06:23:52PM +0100, Dave Gordon wrote:
> On 20/04/15 21:33, Chris Wilson wrote:
> > On Mon, Apr 20, 2015 at 01:09:18PM -0700, Yu Dai wrote:
> >>
> >> On 04/20/2015 12:52 PM, Chris Wilson wrote:
> >>> On Mon, Apr 20, 2015 at 09:02:20AM -0700, Yu Dai wrote:
> >>>>
> >>>> On 04/18/2015 06:47 AM, Chris Wilson wrote:
> >>>>> On Fri, Apr 17, 2015 at 02:21:12PM -0700, yu.dai at intel.com wrote:
> >>>>>> From: Alex Dai <yu.dai at intel.com>
> >>>>>>
> >>>>>> All gem objects used by GuC are pinned to ggtt space out of range
> >>>>>> [0, WOPCM size]. In GuC address space mapping, [0, WPOCM size] is
> >>>>>> used internally for its Boot ROM, SRAM etc. Currently this WPOCM
> >>>>>> size is 512K. This is done by using of PIN_OFFSET_BIAS.
> >>>>>
> >>>>> If the region is reserved, remove that region from the GGTT drm_mm range
> >>>>> manager. Then the restriction is applied to all objects and not in a
> >>>>> hodge-podge fashion like this.
> >>>>>
> >>>> I don't think I have clearly explained this. GTT range [0, WPOCM
> >>>> size] can't be used by GuC firmware, but still others can use it
> >>>> without any issue. PIN_OFFSET_BIAS is great for such use case.
> >>>
> >>> You mean that the GuC redirects the [0, WOPCM] range to an internal set
> >>> of preallocated PTEs?
> >>>
> >> There is no preallocated PTEs. But GuC treats address within that
> >> range as the Boot ROM or micro-kernel code / data that resides in
> >> its own SRAM. Only when it receives address above WPOCM, it will go
> >> through GGTT to access DRAM memory.
> > 
> > Then I agree your original explanation was very confusing. :)
> > 
> > For the actual code, can you allocate from stolen memory rather than
> > system memory? Also the call to get_pages() is redundant, and by itself
> > misleading since the pages will only be valid whilst pinned which is
> > only done indirectly here.
> > -Chris
> 
> All objects to be shared between the CPU and the GuC must:
> * be permanently resident in real memory, not paged out to shmfs

Look at my suggested create_internal() as an alternative allocator then.

> * be permanently mapped into the GGTT at addresses above WOPCM
> 
> Pinning the object into the GGTT will necessarily result in it
> being kept resident in main memory (i915_gem_object_bind_to_vm()
> calls _get_pages() and then _pin_pages()).
> 
> So we can just lose the _get_pages() call here.
> 
> We have three users of this function:
> * the GuC context pool (one per system)
> * the GuC log (one per system)
> * GuC clients (currently two, maybe more eventually)
> 
> The CPU accesses the context pool only via SG copy operations
> (BTW, there's a bug there, which I'll describe separately) so
> it doesn't need to be directly addressable.
> 
> The GuC log is only accessed via debugfs, which uses kmap_atomic()
> on each page in turn.
> 
> The client data is updated every time we pass work to the GuC;
> at the moment this is also done with kmap_atomic(), but maybe
> the mapping could be set up and left in place instead.

Yes, please look over my suggested performance improvements to fix up
execlists submission. Keeping the kmap whilst the client is active is a
big one.
 
> Do the different access patterns make any difference when we
> choose what to (try to) allocate from stolen?

Yes. Because of a silly hw design we cannot have direct CPU access to
stolen memory. Also if the kernel only accesses through kmapping, then
can we pin the objects high in the GGTT above the aperture?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre