drm/i915/guc: Reserve the upper end of the Global GTT for the GuC
diff mbox

Message ID 9976488e-c41d-a9a5-ee0d-768ff4fe10b9@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio Dec. 22, 2016, 6:44 p.m. UTC
On 22/12/16 08:38, Arkadiusz Hiler wrote:
> On Thu, Dec 22, 2016 at 03:18:08PM +0000, Chris Wilson wrote:
>> On Thu, Dec 22, 2016 at 03:53:15PM +0100, Arkadiusz Hiler wrote:
>>> On Wed, Dec 21, 2016 at 07:35:04PM +0100, Srivatsa, Anusha wrote:
>>>> With enable_guc_loading=2 and enable_guc_submission=0 I get HuC
>>>> authentication failure and with enable_guc_loading and
>>>> enable_guc_submisssion both set to 2 the device does not show any
>>>> display.
>>>
>>> Sadly "the fix" fixes the issues we had with the loading - we can load
>>> GuC but...
>>>
>>> GuC Actions (INTEL_GUC_ACTION*, and intel_guc_send()) seem to care about
>>> kernel context pinned location as well (not directly though). And it's
>>> much more restritive.
>>>
>>> Experimentally I was able to determine that if we have ctx pinned above
>>> 0x10000000 (2 GB) mark, GuC Actions just fail.
>>
>> Are we certain this only applies to the kernel/golden context? No other
>> objects accessed by the GuC are affected?
>> -Chris
>
> Not sure yet.
>

I've root caused the issue to the ring being placed in the bottom area 
of the GGTT (GuC validates the ring offset as well). It looks like the 
allocator will place the ring offset immediately after the ctx if the 
latter is placed at an offset below 0x10000000 and will instead restart 
from the bottom of the GGTT otherwise.

Without Chris' patch to pin the context at the top:
CTX: 92KiB 01 01 LLC dirty (pinned x 1) (ggtt offset: 00080000, size: 
00017000, type: 0)
RING:16KiB 40 40 LLC dirty (pinned x 1) (ggtt offset: 00097000, size: 
00004000, type: 0) (stolen: 00002000)

With the patch:
CTX: 92KiB 01 01 LLC dirty (pinned x 1) (ggtt offset: fede6000, size: 
00017000, type: 0)
RING:16KiB 40 40 LLC dirty (pinned x 1) (ggtt offset: 00001000, size: 
00004000, type: 0) (stolen: 00002000)

Forcing the ring to be above GUC_WOPCM_TOP fixed both HuC loading and 
host2guc with GuC submission for me (see diff below). Can someone verify?

Daniele




>> --
>> Chris Wilson, Intel Open Source Technology Centre
>

Comments

Anusha Srivatsa Dec. 22, 2016, 7:23 p.m. UTC | #1
>-----Original Message-----

>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of

>Daniele Ceraolo Spurio

>Sent: Thursday, December 22, 2016 10:45 AM

>To: intel-gfx@lists.freedesktop.org

>Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc: Reserve the upper end of the

>Global GTT for the GuC

>

>

>On 22/12/16 08:38, Arkadiusz Hiler wrote:

>> On Thu, Dec 22, 2016 at 03:18:08PM +0000, Chris Wilson wrote:

>>> On Thu, Dec 22, 2016 at 03:53:15PM +0100, Arkadiusz Hiler wrote:

>>>> On Wed, Dec 21, 2016 at 07:35:04PM +0100, Srivatsa, Anusha wrote:

>>>>> With enable_guc_loading=2 and enable_guc_submission=0 I get HuC

>>>>> authentication failure and with enable_guc_loading and

>>>>> enable_guc_submisssion both set to 2 the device does not show any

>>>>> display.

>>>>

>>>> Sadly "the fix" fixes the issues we had with the loading - we can

>>>> load GuC but...

>>>>

>>>> GuC Actions (INTEL_GUC_ACTION*, and intel_guc_send()) seem to care

>>>> about kernel context pinned location as well (not directly though).

>>>> And it's much more restritive.

>>>>

>>>> Experimentally I was able to determine that if we have ctx pinned

>>>> above

>>>> 0x10000000 (2 GB) mark, GuC Actions just fail.

>>>

>>> Are we certain this only applies to the kernel/golden context? No

>>> other objects accessed by the GuC are affected?

>>> -Chris

>>

>> Not sure yet.

>>

>

>I've root caused the issue to the ring being placed in the bottom area of the GGTT

>(GuC validates the ring offset as well). It looks like the allocator will place the ring

>offset immediately after the ctx if the latter is placed at an offset below

>0x10000000 and will instead restart from the bottom of the GGTT otherwise.

>

>Without Chris' patch to pin the context at the top:

>CTX: 92KiB 01 01 LLC dirty (pinned x 1) (ggtt offset: 00080000, size:

>00017000, type: 0)

>RING:16KiB 40 40 LLC dirty (pinned x 1) (ggtt offset: 00097000, size:

>00004000, type: 0) (stolen: 00002000)

>

>With the patch:

>CTX: 92KiB 01 01 LLC dirty (pinned x 1) (ggtt offset: fede6000, size:

>00017000, type: 0)

>RING:16KiB 40 40 LLC dirty (pinned x 1) (ggtt offset: 00001000, size:

>00004000, type: 0) (stolen: 00002000)

>

>Forcing the ring to be above GUC_WOPCM_TOP fixed both HuC loading and

>host2guc with GuC submission for me (see diff below). Can someone verify?


Works!
Anusha

>Daniele

>

>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c

>b/drivers/gpu/drm/i915/intel_ringbuffer.c

>index 69ccf4f..0d944ab 100644

>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c

>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c

>@@ -1830,6 +1830,9 @@ int intel_ring_pin(struct intel_ring *ring)

>                         return ret;

>         }

>

>+       if (HAS_GUC_SCHED(ring->engine->i915))

>+               flags |= PIN_OFFSET_BIAS | GUC_WOPCM_TOP;

>+

>         ret = i915_vma_pin(vma, 0, PAGE_SIZE, flags);

>         if (unlikely(ret))

>                 return ret;

>

>

>

>>> --

>>> Chris Wilson, Intel Open Source Technology Centre

>>

>_______________________________________________

>Intel-gfx mailing list

>Intel-gfx@lists.freedesktop.org

>https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 69ccf4f..0d944ab 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1830,6 +1830,9 @@  int intel_ring_pin(struct intel_ring *ring)
                         return ret;
         }

+       if (HAS_GUC_SCHED(ring->engine->i915))
+               flags |= PIN_OFFSET_BIAS | GUC_WOPCM_TOP;
+
         ret = i915_vma_pin(vma, 0, PAGE_SIZE, flags);
         if (unlikely(ret))
                 return ret;