[PATCH] drm/i915: Re-enable aliasing PPGTT mode.

[PATCH] drm/i915: Re-enable aliasing PPGTT mode.

[Zhi Wang]

Aliasing PPGTT mode is broken due to recent changes. Mostly boot the
system with i915.enable_ppgtt=1 will lead a kernel crash.

This patch fixes this problem by:

- PPGTT page table will not be shrinkable if working under aliasing
PPGTT mode.

- Load the root pointers of the PPGTT page table during the context
initialization, as currently the "LRI PDPs updating" magic only works
under full PPGTT mode and also GVT-g doesn't support LRI PDP updating.

Tested on my SKL NUC box.

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/i915_gem_gtt.c | 15 ++++++++++++---
 drivers/gpu/drm/i915/intel_lrc.c    |  5 +++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 68ecfc1..4e06056 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -754,10 +754,19 @@ static bool gen8_ppgtt_clear_pt(struct i915_address_space *vm,
 
 	GEM_BUG_ON(pte_end > GEN8_PTES);
 
-	bitmap_clear(pt->used_ptes, pte, num_entries);
+	/*
+	 * As there is only one PPGTT page table used to mirror the GGTT
+	 * space in the system under aliasing PPGTT mode, we don't need
+	 * to shrink it. Leave the PT pages "always used", so the upper
+	 * level page table pages are safe during clear_range().
+	 *
+	 */
+	if (USES_FULL_PPGTT(vm->i915)) {
+		bitmap_clear(pt->used_ptes, pte, num_entries);
 
-	if (bitmap_empty(pt->used_ptes, GEN8_PTES))
-		return true;
+		if (bitmap_empty(pt->used_ptes, GEN8_PTES))
+			return true;
+	}
 
 	pt_vaddr = kmap_px(pt);
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 44a92ea..9575562 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2009,6 +2009,11 @@ static void execlists_init_reg_state(u32 *reg_state,
 		 * other PDP Descriptors are ignored.
 		 */
 		ASSIGN_CTX_PML4(ppgtt, reg_state);
+	} else {
+		ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
+		ASSIGN_CTX_PDP(ppgtt, reg_state, 1);
+		ASSIGN_CTX_PDP(ppgtt, reg_state, 2);
+		ASSIGN_CTX_PDP(ppgtt, reg_state, 3);
 	}
 
 	if (engine->id == RCS) {
-- 
1.9.1

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

Re: [PATCH] drm/i915: Re-enable aliasing PPGTT mode.

[Chris Wilson]

On Mon, Feb 06, 2017 at 03:04:55PM +0800, Zhi Wang wrote:
> Aliasing PPGTT mode is broken due to recent changes. Mostly boot the
> system with i915.enable_ppgtt=1 will lead a kernel crash.
> 
> This patch fixes this problem by:
> 
> - PPGTT page table will not be shrinkable if working under aliasing
> PPGTT mode.

Not that keen on adding yet another special case into the code. In the
patches I sent, the idea was to keep the PD structure from the
aliasing_ppgtt preallocation by keeping the used_pte count positive (and
so keeping the entire tree marked as in-use).
 
> - Load the root pointers of the PPGTT page table during the context
> initialization, as currently the "LRI PDPs updating" magic only works
> under full PPGTT mode and also GVT-g doesn't support LRI PDP updating.

That is incorrect, they are loaded into the context image before
submission. If you want to make sure they are unchanging so that
lite-restore under g-GVT works, that is another matter.
-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: Re-enable aliasing PPGTT mode.

[Zhi Wang]

Hi Chris:
     Thanks for the reply! :P Have you also fixed here with your new ideas?

in intel_lrc:

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;  // ----> check 
if we should get ppgtt from dev_priv->mm.aliasing_ppgtt.

         u32 *reg_state = ce->lrc_reg_state;

         reg_state[CTX_RING_TAIL+1] = rq->tail;



On 02/06/17 16:10, Chris Wilson wrote:
>> - Load the root pointers of the PPGTT page table during the context
>> >initialization, as currently the "LRI PDPs updating" magic only works
>> >under full PPGTT mode and also GVT-g doesn't support LRI PDP updating.
> That is incorrect, they are loaded into the context image before
> submission. If you want to make sure they are unchanging so that
> lite-restore under g-GVT works, that is another matter.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Re-enable aliasing PPGTT mode.

[Chris Wilson]

On Mon, Feb 06, 2017 at 04:18:23PM +0800, Zhi Wang wrote:
> Hi Chris:
>     Thanks for the reply! :P Have you also fixed here with your new ideas?
> 
> in intel_lrc:
> 
> 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;  // ---->
> check if we should get ppgtt from dev_priv->mm.aliasing_ppgtt.

Oh dear. Yup, that's a bug.

Can you send a ppgtt = rq->ctx->ppgtt ?: &rq->i915->mm.aliasing_ppgtt;

One day we will bite the bullet and set ctx->ppgtt to be the
aliasing_ppgtt (and maybe even fix it so that we can global_gtt as
appropriate), and then just fix up the more unusual case in execbuf to
bind using vm = global_gtt instead (if aliasing).
-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: warning for drm/i915: Re-enable aliasing PPGTT mode.

[Patchwork]

== Series Details ==

Series: drm/i915: Re-enable aliasing PPGTT mode.
URL   : https://patchwork.freedesktop.org/series/19146/
State : warning

== Summary ==

Series 19146v1 drm/i915: Re-enable aliasing PPGTT mode.
https://patchwork.freedesktop.org/api/1.0/series/19146/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-edid:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup force-load-detect:
                dmesg-warn -> PASS       (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-ivb-3770)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-bsw-n3050)

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:209  dwarn:1   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:200  dwarn:1   dfail:0   fail:0   skip:51 
fi-snb-2600      total:252  pass:200  dwarn:0   dfail:0   fail:0   skip:52 

8fdcb61761c51b84390564368cfcdf7e1dadd5a8 drm-tip: 2017y-02m-05d-17h-53m-10s UTC integration manifest
735f29a drm/i915: Re-enable aliasing PPGTT mode.

== Logs ==

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

Re: [PATCH] drm/i915: Re-enable aliasing PPGTT mode.

[Zhi Wang]

Yep. Wait a second. :P

On 02/06/17 16:50, Chris Wilson wrote:
> On Mon, Feb 06, 2017 at 04:18:23PM +0800, Zhi Wang wrote:
>> Hi Chris:
>>      Thanks for the reply! :P Have you also fixed here with your new ideas?
>>
>> in intel_lrc:
>>
>> 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;  // ---->
>> check if we should get ppgtt from dev_priv->mm.aliasing_ppgtt.
>
> Oh dear. Yup, that's a bug.
>
> Can you send a ppgtt = rq->ctx->ppgtt ?: &rq->i915->mm.aliasing_ppgtt;
>
> One day we will bite the bullet and set ctx->ppgtt to be the
> aliasing_ppgtt (and maybe even fix it so that we can global_gtt as
> appropriate), and then just fix up the more unusual case in execbuf to
> bind using vm = global_gtt instead (if aliasing).
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx