On 13/04/16 10:04, Chris Wilson wrote:
On Wed, Apr 13, 2016 at 09:45:03AM +0100, Tvrtko Ursulin wrote:
On 12/04/16 17:21, Chris Wilson wrote:
On Tue, Apr 12, 2016 at 04:56:51PM +0100, Tvrtko Ursulin wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
By tracking the iomapping on the VMA itself, we can share that area
between multiple users. Also by only revoking the iomapping upon
unbinding from the mappable portion of the GGTT, we can keep that iomap
across multiple invocations (e.g. execlists context pinning).
v2:
* Rebase on nightly;
* added kerneldoc. (Tvrtko Ursulin)
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 2 ++
drivers/gpu/drm/i915/i915_gem_gtt.c | 38 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_gem_gtt.h | 38 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_fbdev.c | 8 +++-----
4 files changed, 81 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b37ffea8b458..6a485630595e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3393,6 +3393,8 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
ret = i915_gem_object_put_fence(obj);
if (ret)
return ret;
+
+ i915_vma_iounmap(vma);
}
trace_i915_vma_unbind(vma);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c5cb04907525..b2a8a14e8dcb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3626,3 +3626,41 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
return obj->base.size;
}
}
+
+void *i915_vma_iomap(struct drm_i915_private *dev_priv,
+ struct i915_vma *vma)
+{
+ struct drm_i915_gem_object *obj = vma->obj;
+ struct i915_ggtt *ggtt = &dev_priv->ggtt;
+
+ if (WARN_ON(!obj->map_and_fenceable))
+ return ERR_PTR(-ENODEV);
+
+ BUG_ON(!vma->is_ggtt);
+ BUG_ON(vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL);
+ BUG_ON((vma->bound & GLOBAL_BIND) == 0);
+
+ if (vma->iomap == NULL) {
+ void *ptr;
We could extract ggtt from the is_ggtt vma->vm that would remove the
dev_priv parameter and make the callers a bit tidier.
static inline struct i915_ggtt *to_ggtt(struct i915_address_space *vm)
{
BUG_ON(!i915_is_ggtt(vm));
return container_of(vm, struct i915_ggtt, base);
}
+
+ ptr = ioremap_wc(ggtt->mappable_base + vma->node.start,
+ obj->base.size);
+ if (ptr == NULL) {
+ int ret;
+
+ /* Too many areas already allocated? */
+ ret = i915_gem_evict_vm(vma->vm, true);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ptr = ioremap_wc(ggtt->mappable_base + vma->node.start,
+ obj->base.size);
No, we really don't want to create a new ioremap for every caller when
we already have one, ggtt->mappable. Hence,
io-mapping: Specify mapping size for io_mapping_map_wc
being its preceeding patch. The difference is huge on Braswell.
I was following the principle of least change for this one since it
does remain the same in that respect as the current code. Goal was
to unblock the GuC early unpin / execlist no retired queue series
and not get into the trap of inflating the dependencies too much. I
thought to add this and default context cleanup but you keep adding
things to the pile. :)
Tip of the iceberg. Only we have lots of icebergs. And the occasional
dragon.
I'll have a look at your io_mapping stuff to see how much it would
add to the series. Remember I said I'll add the stable ctx id
patches as well. Do we need to come up with a plan here?
We more or less own io_mapping, for this it is just one patch to add a
pass-through parameter to ioremap_wc that's required for 32bit.
I could mutter about the patch before this being quite a major
bugfix...
I could have two separate series to simplify dependencies a bit:
1. GuC premature unpin and
2. execlist no retired queue.
The stack for the first one could look like (not as patches but
conceptually):
1. default context cleanup
2. any io_mapping patches of yours
3. i915_vma_iomap or WC pin_map as you suggested in the other reply.
4. previous / pinned context
Execlist no retired queue would then be:
1. stable ctx id
2. removal of i915_gem_request_unreference__unlocked
3. execlist no retired queue
If I did not forget about something.
At any point in any series we could add things like
i915_gem_request.c or those patches which move around the context
init.
Could I see you some drm_mm.c patches after that?
If I am competent enough to help with them sure.
Does this mean above sounds like a plan to you? Two series containing
the listed items?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
