[PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Zhi Wang]

execlist_update_context() will try to update PDPs in a context before a
ELSP submission only for full PPGTT mode, while PDPs was populated during
context initialization. Now the latter code path is removed. Let
execlist_update_context() also cover !FULL_PPGTT mode.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 44a92ea..9084337 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -321,7 +321,8 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
 static u64 execlists_update_context(struct drm_i915_gem_request *rq)
 {
 	struct intel_context *ce = &rq->ctx->engine[rq->engine->id];
-	struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
+	struct i915_hw_ppgtt *ppgtt =
+		rq->ctx->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
 	u32 *reg_state = ce->lrc_reg_state;
 
 	reg_state[CTX_RING_TAIL+1] = rq->tail;
-- 
1.9.1

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

Re: [PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Chris Wilson]

On Mon, Feb 06, 2017 at 06:37:16PM +0800, Zhi Wang wrote:
> execlist_update_context() will try to update PDPs in a context before a
> ELSP submission only for full PPGTT mode, while PDPs was populated during
> context initialization. Now the latter code path is removed. Let
> execlist_update_context() also cover !FULL_PPGTT mode.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

My mistake from earlier missing the aliasing case.

Fixes: 34869776c76b ("drm/i915: check ppgtt validity when init reg state")
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Zhi Wang]

Thanks for the merging. I guess that with your patches of keeping PD 
structure under aliasing PPGTT mode in 32bit page table, the amount of 
PDPs will not change anymore under aliasing PPGTT mode. :P

On 02/06/17 18:58, Chris Wilson wrote:
> On Mon, Feb 06, 2017 at 06:37:16PM +0800, Zhi Wang wrote:
>> execlist_update_context() will try to update PDPs in a context before a
>> ELSP submission only for full PPGTT mode, while PDPs was populated during
>> context initialization. Now the latter code path is removed. Let
>> execlist_update_context() also cover !FULL_PPGTT mode.
>>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>
> My mistake from earlier missing the aliasing case.
>
> Fixes: 34869776c76b ("drm/i915: check ppgtt validity when init reg state")
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Chris Wilson]

On Mon, Feb 06, 2017 at 07:03:39PM +0800, Zhi Wang wrote:
> Thanks for the merging. I guess that with your patches of keeping PD
> structure under aliasing PPGTT mode in 32bit page table, the amount
> of PDPs will not change anymore under aliasing PPGTT mode. :P

Yes. That's the idea atm, we will preallocate the aliasing table
and then keep the tree intact. The only disadvantage with gen8+ aliasing
mode (compared to gen6) will be that we still have to walk the va range
to pin the tree so that when we call clear_range afterwards we don't
reap. The benefit from my pov, is that the special case for
aliasing_ppgtt is within the aliasing_ppgtt code and not in the common
code (which should be optimised for full-ppgtt as that is the
default/typical use).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Patchwork]

== Series Details ==

Series: drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
URL   : https://patchwork.freedesktop.org/series/19155/
State : success

== Summary ==

Series 19155v1 drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
https://patchwork.freedesktop.org/api/1.0/series/19155/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-load-detect:
                dmesg-warn -> PASS       (fi-snb-2520m)

fi-bdw-5557u     total:252  pass:214  dwarn:0   dfail:0   fail:0   skip:38 
fi-bsw-n3050     total:252  pass:192  dwarn:0   dfail:0   fail:0   skip:60 
fi-bxt-j4205     total:252  pass:208  dwarn:0   dfail:0   fail:0   skip:44 
fi-bxt-t5700     total:209  pass:167  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-j1900     total:252  pass:204  dwarn:0   dfail:0   fail:0   skip:48 
fi-byt-n2820     total:252  pass:200  dwarn:0   dfail:0   fail:0   skip:52 
fi-hsw-4770      total:252  pass:211  dwarn:0   dfail:0   fail:0   skip:41 
fi-hsw-4770r     total:252  pass:211  dwarn:0   dfail:0   fail:0   skip:41 
fi-ilk-650       total:14   pass:11   dwarn:0   dfail:0   fail:0   skip:2  
fi-ivb-3520m     total:252  pass:210  dwarn:0   dfail:0   fail:0   skip:42 
fi-ivb-3770      total:252  pass:210  dwarn:0   dfail:0   fail:0   skip:42 
fi-kbl-7500u     total:252  pass:207  dwarn:0   dfail:0   fail:2   skip:43 
fi-skl-6260u     total:252  pass:215  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6700hq    total:252  pass:210  dwarn:0   dfail:0   fail:0   skip:42 
fi-skl-6700k     total:252  pass:205  dwarn:4   dfail:0   fail:0   skip:43 
fi-skl-6770hq    total:252  pass:215  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2520m     total:252  pass:201  dwarn:0   dfail:0   fail:0   skip:51 
fi-snb-2600      total:252  pass:200  dwarn:0   dfail:0   fail:0   skip:52 

e7d4ec79b24f82431f34d070d16772538422fb13 drm-tip: 2017y-02m-06d-11h-49m-42s UTC integration manifest
feff831 drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3709/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Chris Wilson]

On Mon, Feb 06, 2017 at 01:54:52PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
> URL   : https://patchwork.freedesktop.org/series/19155/
> State : success
> 
> == Summary ==
> 
> Series 19155v1 drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
> https://patchwork.freedesktop.org/api/1.0/series/19155/revisions/1/mbox/
> 
> Test kms_force_connector_basic:
>         Subgroup force-load-detect:
>                 dmesg-warn -> PASS       (fi-snb-2520m)

Thanks for the fix, pushed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Zhi Wang]

Hi Chris:
     Thanks for the explanation! :P Have you already sent the patch to 
keep PD structure under aliasing PPGTT mode? I tried drm-intel-nightly 
branch and still got kernel panic under aliasing PPGTT mode. T_T

I fixed it like this, is this acceptable as a hot fix? If it's 
acceptable as a hot fix, I can send a patch. :P

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 22b3374..21e06c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2658,6 +2658,19 @@ static int aliasing_gtt_bind_vma(struct i915_vma 
*vma,
  	if (vma->obj->gt_ro)
  		pte_flags |= PTE_READ_ONLY;

+	if (flags & I915_VMA_LOCAL_BIND) {
+		struct i915_hw_ppgtt *appgtt = i915->mm.aliasing_ppgtt;
+
+		ret = appgtt->base.allocate_va_range(&appgtt->base,
+						     vma->node.start,
+						     vma->node.size);
+		if (ret)
+			return ret;
+
+		appgtt->base.insert_entries(&appgtt->base,
+					    vma->pages, vma->node.start,
+					    cache_level, pte_flags);
+	}

  	if (flags & I915_VMA_GLOBAL_BIND) {
  		intel_runtime_pm_get(i915);
@@ -2666,14 +2679,6 @@ static int aliasing_gtt_bind_vma(struct i915_vma 
*vma,
  					cache_level, pte_flags);
  		intel_runtime_pm_put(i915);
  	}
-
-	if (flags & I915_VMA_LOCAL_BIND) {
-		struct i915_hw_ppgtt *appgtt = i915->mm.aliasing_ppgtt;
-		appgtt->base.insert_entries(&appgtt->base,
-					    vma->pages, vma->node.start,
-					    cache_level, pte_flags);
-	}
-
  	return 0;
  }



On 02/06/17 19:38, Chris Wilson wrote:
> On Mon, Feb 06, 2017 at 07:03:39PM +0800, Zhi Wang wrote:
>> Thanks for the merging. I guess that with your patches of keeping PD
>> structure under aliasing PPGTT mode in 32bit page table, the amount
>> of PDPs will not change anymore under aliasing PPGTT mode. :P
>
> Yes. That's the idea atm, we will preallocate the aliasing table
> and then keep the tree intact. The only disadvantage with gen8+ aliasing
> mode (compared to gen6) will be that we still have to walk the va range
> to pin the tree so that when we call clear_range afterwards we don't
> reap. The benefit from my pov, is that the special case for
> aliasing_ppgtt is within the aliasing_ppgtt code and not in the common
> code (which should be optimised for full-ppgtt as that is the
> default/typical use).
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.

[Chris Wilson]

On Tue, Feb 07, 2017 at 05:22:33PM +0800, Zhi Wang wrote:
> Hi Chris:
>     Thanks for the explanation! :P Have you already sent the patch
> to keep PD structure under aliasing PPGTT mode? I tried
> drm-intel-nightly branch and still got kernel panic under aliasing
> PPGTT mode. T_T

It's on the list, just starting to get reviews.
> 
> I fixed it like this, is this acceptable as a hot fix? If it's
> acceptable as a hot fix, I can send a patch. :P

Is one of the patches. The only difference is that we don't actually
want to run alloc_va_range for gen6/7 as that is static. A minor detail.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx