[2/2] drm/i915: re-use computed offset bias for context pin
diff mbox

Message ID 1482537382-28584-2-git-send-email-daniele.ceraolospurio@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio Dec. 23, 2016, 11:56 p.m. UTC
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

The context has to obey the same offset requirements as the ring,
so we can re-use the same bias value we computed for the ring instead of
unconditionally using GUC_WOPCM_TOP.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Chris Wilson Dec. 24, 2016, 10:12 a.m. UTC | #1
On Fri, Dec 23, 2016 at 03:56:22PM -0800, daniele.ceraolospurio@intel.com wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> The context has to obey the same offset requirements as the ring,
> so we can re-use the same bias value we computed for the ring instead of
> unconditionally using GUC_WOPCM_TOP.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Both 
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Applied and pushed. Thanks for digging in to find the root cause. We
could relax the 4096 offset for bdw+ I think, or maybe only gen6/gen7
had enough coverage in the wild to hit the hole.
-Chris
Daniele Ceraolo Spurio Dec. 24, 2016, 6:40 p.m. UTC | #2
On 12/24/2016 2:12 AM, Chris Wilson wrote:
> On Fri, Dec 23, 2016 at 03:56:22PM -0800, daniele.ceraolospurio@intel.com wrote:
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> The context has to obey the same offset requirements as the ring,
>> so we can re-use the same bias value we computed for the ring instead of
>> unconditionally using GUC_WOPCM_TOP.
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Both
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Applied and pushed. Thanks for digging in to find the root cause. We
> could relax the 4096 offset for bdw+ I think, or maybe only gen6/gen7
> had enough coverage in the wild to hit the hole.
> -Chris
>

Thanks for the review :-)
We still need your patch to avoid putting any guc related object above
0xFEE00000 to fix the other side of the issue, are you planning a respin
of that?

Thanks,
Daniele

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cec9037..0a321d1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,7 +767,7 @@  static int execlists_context_pin(struct intel_engine_cs *engine,
 				 struct i915_gem_context *ctx)
 {
 	struct intel_context *ce = &ctx->engine[engine->id];
-	unsigned int flags;
+	unsigned int flags = PIN_GLOBAL;
 	void *vaddr;
 	int ret;
 
@@ -782,7 +782,9 @@  static int execlists_context_pin(struct intel_engine_cs *engine,
 			goto err;
 	}
 
-	flags = PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL;
+	if (ctx->ggtt_offset_bias)
+		flags |= PIN_OFFSET_BIAS | ctx->ggtt_offset_bias;
+
 	if (ctx == ctx->i915->kernel_context)
 		flags |= PIN_HIGH;