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

From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, Yu Dai <yu.dai@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 07/15] drm/i915: Add functions to allocate / release gem obj for GuC
Date: Tue, 21 Apr 2015 18:23:52 +0100	[thread overview]
Message-ID: <553687A8.6000604@intel.com> (raw)
In-Reply-To: <20150420203353.GA11019@nuc-i3427.alporthouse.com>

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@intel.com wrote:
>>>>>> From: Alex Dai <yu.dai@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
* 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.

Do the different access patterns make any difference when we
choose what to (try to) allocate from stolen?

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx