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

Submitted by yu.dai@intel.com on April 17, 2015, 9:21 p.m.

Details

Message ID 1429305680-4990-8-git-send-email-yu.dai@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

yu.dai@intel.com April 17, 2015, 9:21 p.m.
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.

Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        |  3 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 55 +++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 0a73122..6b2b5bf 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -99,5 +99,8 @@  struct intel_guc {
 extern int intel_guc_load_ucode(struct drm_device *dev, bool wait);
 extern void intel_guc_ucode_fini(struct drm_device *dev);
 extern void intel_guc_ucode_init(struct drm_device *dev);
+struct drm_i915_gem_object *
+intel_guc_allocate_gem_obj(struct drm_device *dev, u32 size);
+void intel_guc_release_gem_obj(struct drm_i915_gem_object *obj);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index a17f210..04c2b11 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -40,6 +40,12 @@ 
  * The firmware installation package will install (symbolic link) proper version
  * of firmware.
  *
+ * GuC address space:
+ * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
+ * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
+ * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
+ * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
+ *
  */
 
 #define I915_SKL_GUC_UCODE "i915/skl_guc_ver1.bin"
@@ -47,6 +53,55 @@  MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
 #define I915_BXT_GUC_UCODE "i915/bxt_guc_ver1.bin"
 MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
 
+/**
+ * intel_guc_allocate_gem_obj() - Allocate gem object for GuC usage
+ * @dev:	drm device
+ * @size:	size of object
+ *
+ * This is a wrapper to create a gem obj. In order to use it inside GuC, the
+ * object needs to be pinned lifetime. Also we must pin it to gtt space other
+ * than [0, GUC_WOPCM_SIZE] because this range is reserved inside GuC.
+ *
+ * Return:	A drm_i915_gem_object if successful, otherwise NULL.
+ */
+struct drm_i915_gem_object *
+intel_guc_allocate_gem_obj(struct drm_device *dev, u32 size)
+{
+	struct drm_i915_gem_object *obj;
+
+	obj = i915_gem_alloc_object(dev, size);
+	if (!obj)
+		return NULL;
+
+	if (i915_gem_object_get_pages(obj)) {
+		drm_gem_object_unreference(&obj->base);
+		return NULL;
+	}
+
+	if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
+			PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) {
+		drm_gem_object_unreference(&obj->base);
+		return NULL;
+	}
+
+	return obj;
+}
+
+/**
+ * intel_guc_release_gem_obj() - Release gem object allocated for GuC usage
+ * @obj:	gem obj to be released
+  */
+void intel_guc_release_gem_obj(struct drm_i915_gem_object *obj)
+{
+	if (!obj)
+		return;
+
+	if (i915_gem_obj_is_pinned(obj))
+		i915_gem_object_ggtt_unpin(obj);
+
+	drm_gem_object_unreference(&obj->base);
+}
+
 /* Read GuC status register (GUC_STATUS)
  * Return true if get a success code from normal boot or RC6 boot
  */

Comments

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.
-Chris
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.
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?
-Chris
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.

Thanks,
Alex
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
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.
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@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

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