public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gtt: Clear range always uses scratch
@ 2016-05-25 13:50 Mika Kuoppala
  2016-05-25 14:00 ` Chris Wilson
  2016-05-25 14:37 ` ✗ Ro.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 4+ messages in thread
From: Mika Kuoppala @ 2016-05-25 13:50 UTC (permalink / raw)
  To: intel-gfx

As clear_range uses scratch page on all callsites, remove
the superfluous parameter.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 46 ++++++++++++++-----------------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +--
 2 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 46684779d4d6..3d1e704414f1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -184,8 +184,7 @@ static void ppgtt_unbind_vma(struct i915_vma *vma)
 {
 	vma->vm->clear_range(vma->vm,
 			     vma->node.start,
-			     vma->obj->base.size,
-			     true);
+			     vma->obj->base.size);
 }
 
 static gen8_pte_t gen8_pte_encode(dma_addr_t addr,
@@ -769,12 +768,11 @@ static void gen8_ppgtt_clear_pte_range(struct i915_address_space *vm,
 
 static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
 				   uint64_t start,
-				   uint64_t length,
-				   bool use_scratch)
+				   uint64_t length)
 {
 	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
 	gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
-						 I915_CACHE_LLC, use_scratch);
+						 I915_CACHE_LLC, true);
 
 	if (!USES_FULL_48BIT_PPGTT(vm->dev)) {
 		gen8_ppgtt_clear_pte_range(vm, &ppgtt->pdp, start, length,
@@ -1801,8 +1799,7 @@ static void gen6_ppgtt_enable(struct drm_device *dev)
 /* PPGTT support for Sandybdrige/Gen6 and later */
 static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 				   uint64_t start,
-				   uint64_t length,
-				   bool use_scratch)
+				   uint64_t length)
 {
 	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
 	gen6_pte_t *pt_vaddr, scratch_pte;
@@ -2329,8 +2326,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
 
 	i915_check_and_clear_faults(dev_priv);
 
-	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total,
-			     true);
+	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total);
 
 	i915_ggtt_flush(dev_priv);
 }
@@ -2474,15 +2470,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 
 static void nop_clear_range(struct i915_address_space *vm,
 			    uint64_t start,
-			    uint64_t length,
-			    bool use_scratch)
+			    uint64_t length)
 {
 }
 
 static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
-				  uint64_t length,
-				  bool use_scratch)
+				  uint64_t length)
 {
 	struct drm_i915_private *dev_priv = to_i915(vm->dev);
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
@@ -2503,7 +2497,7 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 
 	scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
 				      I915_CACHE_LLC,
-				      use_scratch);
+				      true);
 	for (i = 0; i < num_entries; i++)
 		gen8_set_pte(&gtt_base[i], scratch_pte);
 	readl(gtt_base);
@@ -2513,8 +2507,7 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 
 static void gen6_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
-				  uint64_t length,
-				  bool use_scratch)
+				  uint64_t length)
 {
 	struct drm_i915_private *dev_priv = to_i915(vm->dev);
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
@@ -2534,7 +2527,7 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
 		num_entries = max_entries;
 
 	scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
-				     I915_CACHE_LLC, use_scratch, 0);
+				     I915_CACHE_LLC, true, 0);
 
 	for (i = 0; i < num_entries; i++)
 		iowrite32(scratch_pte, &gtt_base[i]);
@@ -2563,8 +2556,7 @@ static void i915_ggtt_insert_entries(struct i915_address_space *vm,
 
 static void i915_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
-				  uint64_t length,
-				  bool unused)
+				  uint64_t length)
 {
 	struct drm_i915_private *dev_priv = vm->dev->dev_private;
 	unsigned first_entry = start >> PAGE_SHIFT;
@@ -2656,8 +2648,7 @@ static void ggtt_unbind_vma(struct i915_vma *vma)
 	if (vma->bound & GLOBAL_BIND) {
 		vma->vm->clear_range(vma->vm,
 				     vma->node.start,
-				     size,
-				     true);
+				     size);
 	}
 
 	if (dev_priv->mm.aliasing_ppgtt && vma->bound & LOCAL_BIND) {
@@ -2665,8 +2656,7 @@ static void ggtt_unbind_vma(struct i915_vma *vma)
 
 		appgtt->base.clear_range(&appgtt->base,
 					 vma->node.start,
-					 size,
-					 true);
+					 size);
 	}
 }
 
@@ -2764,11 +2754,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 		DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
 			      hole_start, hole_end);
 		ggtt->base.clear_range(&ggtt->base, hole_start,
-				     hole_end - hole_start, true);
+				       hole_end - hole_start);
 	}
 
 	/* And finally clear the reserved guard page */
-	ggtt->base.clear_range(&ggtt->base, end - PAGE_SIZE, PAGE_SIZE, true);
+	ggtt->base.clear_range(&ggtt->base, end - PAGE_SIZE, PAGE_SIZE);
 
 	if (USES_PPGTT(dev) && !USES_FULL_PPGTT(dev)) {
 		struct i915_hw_ppgtt *ppgtt;
@@ -2795,8 +2785,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 
 		ppgtt->base.clear_range(&ppgtt->base,
 					ppgtt->base.start,
-					ppgtt->base.total,
-					true);
+					ppgtt->base.total);
 
 		dev_priv->mm.aliasing_ppgtt = ppgtt;
 		WARN_ON(ggtt->base.bind_vma != ggtt_bind_vma);
@@ -3254,8 +3243,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	i915_check_and_clear_faults(dev_priv);
 
 	/* First fill our portion of the GTT with scratch pages */
-	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total,
-			       true);
+	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total);
 
 	/* Cache flush objects bound into GGTT and rebind them. */
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 62be77cac5cd..10d498a2e9a7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -317,8 +317,7 @@ struct i915_address_space {
 				 uint64_t length);
 	void (*clear_range)(struct i915_address_space *vm,
 			    uint64_t start,
-			    uint64_t length,
-			    bool use_scratch);
+			    uint64_t length);
 	void (*insert_entries)(struct i915_address_space *vm,
 			       struct sg_table *st,
 			       uint64_t start,
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/gtt: Clear range always uses scratch
  2016-05-25 13:50 [PATCH] drm/i915/gtt: Clear range always uses scratch Mika Kuoppala
@ 2016-05-25 14:00 ` Chris Wilson
  2016-05-25 14:09   ` Mika Kuoppala
  2016-05-25 14:37 ` ✗ Ro.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2016-05-25 14:00 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Wed, May 25, 2016 at 04:50:14PM +0300, Mika Kuoppala wrote:
> As clear_range uses scratch page on all callsites, remove
> the superfluous parameter.

It's there so that we had a convenient place to enable GPU faults for
out-of-bound access.

But I think that ship has long sailed, as nobody has prepared for
batchbuffers to be restricted in length.

So,
Reviewed-by: Chris Wilson <chris@chris-wilson>

Did you have any clear motiviation? Anything positive we can spin from
this?
-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] 4+ messages in thread
* Re: [PATCH] drm/i915/gtt: Clear range always uses scratch
  2016-05-25 14:00 ` Chris Wilson
@ 2016-05-25 14:09   ` Mika Kuoppala
  0 siblings, 0 replies; 4+ messages in thread
From: Mika Kuoppala @ 2016-05-25 14:09 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> [ text/plain ]
> On Wed, May 25, 2016 at 04:50:14PM +0300, Mika Kuoppala wrote:
>> As clear_range uses scratch page on all callsites, remove
>> the superfluous parameter.
>
> It's there so that we had a convenient place to enable GPU faults for
> out-of-bound access.
>
> But I think that ship has long sailed, as nobody has prepared for
> batchbuffers to be restricted in length.
>
> So,
> Reviewed-by: Chris Wilson <chris@chris-wilson>
>
> Did you have any clear motiviation? Anything positive we can spin from
> this?

No clear motivation, just bumped into this when reading code and
trying to figure out if WaDisableNullPageAsDummy is needed or not.

Looks like we always have a valid pte underneath and thus the
pte encodings will be always called as valid == true.
But I stopped at here as I don't know if virtualization/svm
will take use of non valid ones.

-Mika

> -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] 4+ messages in thread
* ✗ Ro.CI.BAT: failure for drm/i915/gtt: Clear range always uses scratch
  2016-05-25 13:50 [PATCH] drm/i915/gtt: Clear range always uses scratch Mika Kuoppala
  2016-05-25 14:00 ` Chris Wilson
@ 2016-05-25 14:37 ` Patchwork
  1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2016-05-25 14:37 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gtt: Clear range always uses scratch
URL   : https://patchwork.freedesktop.org/series/7725/
State : failure

== Summary ==

Series 7725v1 drm/i915/gtt: Clear range always uses scratch
http://patchwork.freedesktop.org/api/1.0/series/7725/revisions/1/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
                fail       -> PASS       (fi-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i7-5600u)
                skip       -> PASS       (fi-skl-i5-6260u)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:2   skip:38 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:198  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5557U  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
ro-bdw-i7-5600u  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1009/

fc9d741 drm-intel-nightly: 2016y-05m-25d-07h-45m-48s UTC integration manifest
b3e5f4d drm/i915/gtt: Clear range always uses scratch

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-25 14:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-25 13:50 [PATCH] drm/i915/gtt: Clear range always uses scratch Mika Kuoppala
2016-05-25 14:00 ` Chris Wilson
2016-05-25 14:09   ` Mika Kuoppala
2016-05-25 14:37 ` ✗ Ro.CI.BAT: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox