All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
@ 2017-01-17 14:06 Zhi Wang
  2017-01-17 14:06 ` [PATCH] drm/i915: Re-enable preallocated top level PDPs support Zhi Wang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Zhi Wang @ 2017-01-17 14:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: Zhiyuan Lv

From: Zhenyu Wang <zhenyuw@linux.intel.com>

Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
helper") introduces this typo which can cause a driver loading failure
in Linux GVT-g guest.

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index f1ad4fb..d0abfd0 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -122,7 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
 {
 	unsigned long size = end - start;
 
-	if (start <= end)
+	if (start >= end)
 		return -EINVAL;
 
 	DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n",
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] drm/i915: Re-enable preallocated top level PDPs support
  2017-01-17 14:06 [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Zhi Wang
@ 2017-01-17 14:06 ` Zhi Wang
  2017-01-19 11:52   ` Michał Winiarski
  2017-01-17 14:27 ` [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Zhi Wang @ 2017-01-17 14:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: Zhiyuan Lv

After PPGTT page table is able to be shrinken, the preallocated PDPs and
PDE pages can be freed even they are preallocated under 3-level PPGTT
mode. This patch re-enables preallocated top level PDPs and PDE pages
like before.

Cc: Michał 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 | 10 +++++++++-
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8aca11f..f0e1992 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -793,12 +793,18 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
 	struct i915_page_directory *pd;
 	uint64_t pdpe;
+	bool pd_is_empty;
 
 	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
 		if (WARN_ON(!pdp->page_directory[pdpe]))
 			break;
 
-		if (gen8_ppgtt_clear_pd(vm, pd, start, length)) {
+		pd_is_empty = gen8_ppgtt_clear_pd(vm, pd, start, length);
+		/* Do not free pd pages if pdps are preallocated. */
+		if (ppgtt->preallocate_top_level_pdps)
+			continue;
+
+		if (pd_is_empty) {
 			__clear_bit(pdpe, pdp->used_pdpes);
 			gen8_setup_pdpe(ppgtt, pdp, vm->scratch_pd, pdpe);
 			free_pd(vm->i915, pd);
@@ -1545,6 +1551,8 @@ static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
 
 	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
 
+	ppgtt->preallocate_top_level_pdps = true;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 34a4fd5..f325cb8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -349,6 +349,7 @@ struct i915_hw_ppgtt {
 	struct kref ref;
 	struct drm_mm_node node;
 	unsigned long pd_dirty_rings;
+	bool preallocate_top_level_pdps;
 	union {
 		struct i915_pml4 pml4;		/* GEN8+ & 48b PPGTT */
 		struct i915_page_directory_pointer pdp;	/* GEN8+ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index db714dc..766a91a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1488,7 +1488,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 	if (req->ctx->ppgtt &&
 	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
 		if (!USES_FULL_48BIT_PPGTT(req->i915) &&
-		    !intel_vgpu_active(req->i915)) {
+		    !req->ctx->ppgtt->preallocate_top_level_pdps) {
 			ret = intel_logical_ring_emit_pdps(req);
 			if (ret)
 				return ret;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
  2017-01-17 14:06 [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Zhi Wang
  2017-01-17 14:06 ` [PATCH] drm/i915: Re-enable preallocated top level PDPs support Zhi Wang
@ 2017-01-17 14:27 ` Jani Nikula
  2017-01-17 15:11   ` Chris Wilson
  2017-01-18  5:42   ` Zhi Wang
  2017-01-17 14:37 ` Chris Wilson
  2017-01-17 14:54 ` ✓ Fi.CI.BAT: success for drm/i915: Fix a typo in vgt_balloon_space() (rev2) Patchwork
  3 siblings, 2 replies; 11+ messages in thread
From: Jani Nikula @ 2017-01-17 14:27 UTC (permalink / raw)
  To: Zhi Wang, intel-gfx; +Cc: Zhiyuan Lv

On Tue, 17 Jan 2017, Zhi Wang <zhi.a.wang@intel.com> wrote:
> From: Zhenyu Wang <zhenyuw@linux.intel.com>
>
> Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
> helper") introduces this typo which can cause a driver loading failure
> in Linux GVT-g guest.

The proper format for fixes is:

Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper")

> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index f1ad4fb..d0abfd0 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -122,7 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
>  {
>  	unsigned long size = end - start;
>  
> -	if (start <= end)
> +	if (start >= end)
>  		return -EINVAL;
>  
>  	DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n",

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
  2017-01-17 14:06 [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Zhi Wang
  2017-01-17 14:06 ` [PATCH] drm/i915: Re-enable preallocated top level PDPs support Zhi Wang
  2017-01-17 14:27 ` [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Jani Nikula
@ 2017-01-17 14:37 ` Chris Wilson
  2017-01-18  5:43   ` Zhi Wang
  2017-01-17 14:54 ` ✓ Fi.CI.BAT: success for drm/i915: Fix a typo in vgt_balloon_space() (rev2) Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-01-17 14:37 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, Zhiyuan Lv

On Tue, Jan 17, 2017 at 10:06:11PM +0800, Zhi Wang wrote:
> From: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
> Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
> helper") introduces this typo which can cause a driver loading failure
> in Linux GVT-g guest.
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index f1ad4fb..d0abfd0 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -122,7 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
>  {
>  	unsigned long size = end - start;
>  
> -	if (start <= end)
> +	if (start >= end)
>  		return -EINVAL;

Oops. Sorry,
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Fix a typo in vgt_balloon_space() (rev2)
  2017-01-17 14:06 [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Zhi Wang
                   ` (2 preceding siblings ...)
  2017-01-17 14:37 ` Chris Wilson
@ 2017-01-17 14:54 ` Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-01-17 14:54 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix a typo in vgt_balloon_space() (rev2)
URL   : https://patchwork.freedesktop.org/series/18123/
State : success

== Summary ==

Series 18123v2 drm/i915: Fix a typo in vgt_balloon_space()
https://patchwork.freedesktop.org/api/1.0/series/18123/revisions/2/mbox/


fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

ce0b007247ca4914620056ece367b90adc95fa2f drm-tip: 2017y-01m-17d-12h-37m-22s UTC integration manifest
5aad44c drm/i915: Re-enable preallocated top level PDPs support

== Logs ==

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
  2017-01-17 14:27 ` [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Jani Nikula
@ 2017-01-17 15:11   ` Chris Wilson
  2017-01-18  5:42   ` Zhi Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-01-17 15:11 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Zhiyuan Lv

On Tue, Jan 17, 2017 at 04:27:04PM +0200, Jani Nikula wrote:
> On Tue, 17 Jan 2017, Zhi Wang <zhi.a.wang@intel.com> wrote:
> > From: Zhenyu Wang <zhenyuw@linux.intel.com>
> >
> > Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
> > helper") introduces this typo which can cause a driver loading failure
> > in Linux GVT-g guest.
> 
> The proper format for fixes is:
> 
> Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper")

Pushed the fix for my obvious oops.

Need to spend some more time looking at the pdp fix - it certainly
differs from the patches I have to fixup the same paths.
-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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
  2017-01-17 14:27 ` [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Jani Nikula
  2017-01-17 15:11   ` Chris Wilson
@ 2017-01-18  5:42   ` Zhi Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Zhi Wang @ 2017-01-18  5:42 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Zhiyuan Lv

Thanks, Will update it in V2 :P

于 01/17/17 22:27, Jani Nikula:
> On Tue, 17 Jan 2017, Zhi Wang <zhi.a.wang@intel.com> wrote:
>> From: Zhenyu Wang <zhenyuw@linux.intel.com>
>>
>> Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
>> helper") introduces this typo which can cause a driver loading failure
>> in Linux GVT-g guest.
>
> The proper format for fixes is:
>
> Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper")
>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
>> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
>> index f1ad4fb..d0abfd0 100644
>> --- a/drivers/gpu/drm/i915/i915_vgpu.c
>> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
>> @@ -122,7 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
>>   {
>>   	unsigned long size = end - start;
>>
>> -	if (start <= end)
>> +	if (start >= end)
>>   		return -EINVAL;
>>
>>   	DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n",
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Fix a typo in vgt_balloon_space()
  2017-01-17 14:37 ` Chris Wilson
@ 2017-01-18  5:43   ` Zhi Wang
  0 siblings, 0 replies; 11+ messages in thread
From: Zhi Wang @ 2017-01-18  5:43 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Zhenyu Wang, Joonas Lahtinen, Zhiyuan Lv

It doesn't matter now:P

于 01/17/17 22:37, Chris Wilson:
> On Tue, Jan 17, 2017 at 10:06:11PM +0800, Zhi Wang wrote:
>> From: Zhenyu Wang <zhenyuw@linux.intel.com>
>>
>> Commit 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a
>> helper") introduces this typo which can cause a driver loading failure
>> in Linux GVT-g guest.
>>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
>> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_vgpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
>> index f1ad4fb..d0abfd0 100644
>> --- a/drivers/gpu/drm/i915/i915_vgpu.c
>> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
>> @@ -122,7 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
>>   {
>>   	unsigned long size = end - start;
>>
>> -	if (start <= end)
>> +	if (start >= end)
>>   		return -EINVAL;
>
> Oops. Sorry,
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Re-enable preallocated top level PDPs support
  2017-01-17 14:06 ` [PATCH] drm/i915: Re-enable preallocated top level PDPs support Zhi Wang
@ 2017-01-19 11:52   ` Michał Winiarski
  2017-01-19 12:09     ` Chris Wilson
  2017-01-20  9:11     ` Wang, Zhi A
  0 siblings, 2 replies; 11+ messages in thread
From: Michał Winiarski @ 2017-01-19 11:52 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, Zhiyuan Lv

On Tue, Jan 17, 2017 at 10:06:12PM +0800, Zhi Wang wrote:
> After PPGTT page table is able to be shrinken, the preallocated PDPs and
> PDE pages can be freed even they are preallocated under 3-level PPGTT
> mode. This patch re-enables preallocated top level PDPs and PDE pages
> like before.
> 
> Cc: Michał 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 | 10 +++++++++-
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8aca11f..f0e1992 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -793,12 +793,18 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
>  	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>  	struct i915_page_directory *pd;
>  	uint64_t pdpe;
> +	bool pd_is_empty;
>  
>  	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
>  		if (WARN_ON(!pdp->page_directory[pdpe]))
>  			break;
>  
> -		if (gen8_ppgtt_clear_pd(vm, pd, start, length)) {
> +		pd_is_empty = gen8_ppgtt_clear_pd(vm, pd, start, length);

Why the extra pd_is_empty variable?
Just adding if (preallocate) continue; here is more readable imho.

We should also assert that we're not in 4-level paging mode when shrinking is
skipped.

With those changes:

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> +		/* Do not free pd pages if pdps are preallocated. */
> +		if (ppgtt->preallocate_top_level_pdps)
> +			continue;
> +
> +		if (pd_is_empty) {
>  			__clear_bit(pdpe, pdp->used_pdpes);
>  			gen8_setup_pdpe(ppgtt, pdp, vm->scratch_pd, pdpe);
>  			free_pd(vm->i915, pd);
> @@ -1545,6 +1551,8 @@ static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
>  
>  	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>  
> +	ppgtt->preallocate_top_level_pdps = true;
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 34a4fd5..f325cb8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -349,6 +349,7 @@ struct i915_hw_ppgtt {
>  	struct kref ref;
>  	struct drm_mm_node node;
>  	unsigned long pd_dirty_rings;
> +	bool preallocate_top_level_pdps;
>  	union {
>  		struct i915_pml4 pml4;		/* GEN8+ & 48b PPGTT */
>  		struct i915_page_directory_pointer pdp;	/* GEN8+ */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index db714dc..766a91a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1488,7 +1488,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	if (req->ctx->ppgtt &&
>  	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
>  		if (!USES_FULL_48BIT_PPGTT(req->i915) &&
> -		    !intel_vgpu_active(req->i915)) {
> +		    !req->ctx->ppgtt->preallocate_top_level_pdps) {
>  			ret = intel_logical_ring_emit_pdps(req);
>  			if (ret)
>  				return ret;
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Re-enable preallocated top level PDPs support
  2017-01-19 11:52   ` Michał Winiarski
@ 2017-01-19 12:09     ` Chris Wilson
  2017-01-20  9:11     ` Wang, Zhi A
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-01-19 12:09 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx, Zhiyuan Lv

On Thu, Jan 19, 2017 at 12:52:39PM +0100, Michał Winiarski wrote:
> On Tue, Jan 17, 2017 at 10:06:12PM +0800, Zhi Wang wrote:
> > After PPGTT page table is able to be shrinken, the preallocated PDPs and
> > PDE pages can be freed even they are preallocated under 3-level PPGTT
> > mode. This patch re-enables preallocated top level PDPs and PDE pages
> > like before.
> > 
> > Cc: Michał 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 | 10 +++++++++-
> >  drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
> >  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
> >  3 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 8aca11f..f0e1992 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -793,12 +793,18 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
> >  	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> >  	struct i915_page_directory *pd;
> >  	uint64_t pdpe;
> > +	bool pd_is_empty;
> >  
> >  	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
> >  		if (WARN_ON(!pdp->page_directory[pdpe]))
> >  			break;
> >  
> > -		if (gen8_ppgtt_clear_pd(vm, pd, start, length)) {
> > +		pd_is_empty = gen8_ppgtt_clear_pd(vm, pd, start, length);
> 
> Why the extra pd_is_empty variable?
> Just adding if (preallocate) continue; here is more readable imho.
> 
> We should also assert that we're not in 4-level paging mode when shrinking is
> skipped.

I've restructured this so that we only shrink from clear_range_4lvl.
Having written the tests to exercise your code and put it to use.

Michał can you look at the ppgtt selftests can see if you can think of
any other way we might be able to exercise the alloc/insert/clear?
-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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: Re-enable preallocated top level PDPs support
  2017-01-19 11:52   ` Michał Winiarski
  2017-01-19 12:09     ` Chris Wilson
@ 2017-01-20  9:11     ` Wang, Zhi A
  1 sibling, 0 replies; 11+ messages in thread
From: Wang, Zhi A @ 2017-01-20  9:11 UTC (permalink / raw)
  To: Winiarski, Michal; +Cc: intel-gfx, Lv, Zhiyuan

Hi Guys:
     Please don't merge this patch at this time as I found another problem about alias PPGTT. 

It seems the alias PPGTT on GEN8+ is also broken under native i915. The first allocate_va_range() in alias PPGTT initialization path will allocate all the page tables and the next clear_range() will shrink the allocated page table. It's OK at this time, but when doing VMA binding, alias_ppgtt_bind_vma() will directly insert PTE entries without calling a allocate_va_range() to create the page table first.  This causes an OOPS on my machine.

It looks like we have two options:

1. Let the alias PPGTT page table also be shrinkable.
2. Follow the previous approach, don't shrink the alias PPGTT page table(Probably add some hack in clear_range() path.

-----Original Message-----
From: Winiarski, Michal 
Sent: Thursday, January 19, 2017 7:53 PM
To: Wang, Zhi A <zhi.a.wang@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Thierry, Michel <michel.thierry@intel.com>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Zhenyu Wang <zhenyuw@linux.intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
Subject: Re: [PATCH] drm/i915: Re-enable preallocated top level PDPs support

On Tue, Jan 17, 2017 at 10:06:12PM +0800, Zhi Wang wrote:
> After PPGTT page table is able to be shrinken, the preallocated PDPs 
> and PDE pages can be freed even they are preallocated under 3-level 
> PPGTT mode. This patch re-enables preallocated top level PDPs and PDE 
> pages like before.
> 
> Cc: Michał 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 | 10 +++++++++-  
> drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8aca11f..f0e1992 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -793,12 +793,18 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
>  	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>  	struct i915_page_directory *pd;
>  	uint64_t pdpe;
> +	bool pd_is_empty;
>  
>  	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
>  		if (WARN_ON(!pdp->page_directory[pdpe]))
>  			break;
>  
> -		if (gen8_ppgtt_clear_pd(vm, pd, start, length)) {
> +		pd_is_empty = gen8_ppgtt_clear_pd(vm, pd, start, length);

Why the extra pd_is_empty variable?
Just adding if (preallocate) continue; here is more readable imho.

We should also assert that we're not in 4-level paging mode when shrinking is skipped.

With those changes:

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> +		/* Do not free pd pages if pdps are preallocated. */
> +		if (ppgtt->preallocate_top_level_pdps)
> +			continue;
> +
> +		if (pd_is_empty) {
>  			__clear_bit(pdpe, pdp->used_pdpes);
>  			gen8_setup_pdpe(ppgtt, pdp, vm->scratch_pd, pdpe);
>  			free_pd(vm->i915, pd);
> @@ -1545,6 +1551,8 @@ static int 
> gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
>  
>  	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>  
> +	ppgtt->preallocate_top_level_pdps = true;
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 34a4fd5..f325cb8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -349,6 +349,7 @@ struct i915_hw_ppgtt {
>  	struct kref ref;
>  	struct drm_mm_node node;
>  	unsigned long pd_dirty_rings;
> +	bool preallocate_top_level_pdps;
>  	union {
>  		struct i915_pml4 pml4;		/* GEN8+ & 48b PPGTT */
>  		struct i915_page_directory_pointer pdp;	/* GEN8+ */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index db714dc..766a91a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1488,7 +1488,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	if (req->ctx->ppgtt &&
>  	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
>  		if (!USES_FULL_48BIT_PPGTT(req->i915) &&
> -		    !intel_vgpu_active(req->i915)) {
> +		    !req->ctx->ppgtt->preallocate_top_level_pdps) {
>  			ret = intel_logical_ring_emit_pdps(req);
>  			if (ret)
>  				return ret;
> --
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-01-20  9:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-17 14:06 [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Zhi Wang
2017-01-17 14:06 ` [PATCH] drm/i915: Re-enable preallocated top level PDPs support Zhi Wang
2017-01-19 11:52   ` Michał Winiarski
2017-01-19 12:09     ` Chris Wilson
2017-01-20  9:11     ` Wang, Zhi A
2017-01-17 14:27 ` [PATCH] drm/i915: Fix a typo in vgt_balloon_space() Jani Nikula
2017-01-17 15:11   ` Chris Wilson
2017-01-18  5:42   ` Zhi Wang
2017-01-17 14:37 ` Chris Wilson
2017-01-18  5:43   ` Zhi Wang
2017-01-17 14:54 ` ✓ Fi.CI.BAT: success for drm/i915: Fix a typo in vgt_balloon_space() (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.