* [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
@ 2017-02-17 15:12 Chris Wilson
2017-02-17 15:13 ` [CI 2/6] drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease Chris Wilson
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:12 UTC (permalink / raw)
To: intel-gfx
The object already stores (computed on the fly) the index to dma address
so use it instead of reallocating a large temporary array every time we
bind a rotated framebuffer.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 77 ++++++++++++-------------------------
1 file changed, 25 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 47a38272f54c..848dbb926fd1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3043,27 +3043,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
}
static struct scatterlist *
-rotate_pages(const dma_addr_t *in, unsigned int offset,
- unsigned int width, unsigned int height,
- unsigned int stride,
+rotate_pages(struct drm_i915_gem_object *obj,
+ const struct intel_rotation_plane_info *p,
struct sg_table *st, struct scatterlist *sg)
{
unsigned int column, row;
- unsigned int src_idx;
- for (column = 0; column < width; column++) {
- src_idx = stride * (height - 1) + column;
- for (row = 0; row < height; row++) {
- st->nents++;
+ for (column = 0; column < p->width; column++) {
+ unsigned long src_idx =
+ p->stride * (p->height - 1) + column + p->offset;
+ for (row = 0; row < p->height; row++) {
+ struct scatterlist *src;
+ unsigned int n;
+
+ src = i915_gem_object_get_sg(obj, src_idx, &n);
+ src_idx -= p->stride;
+
/* We don't need the pages, but need to initialize
* the entries so the sg list can be happily traversed.
* The only thing we need are DMA addresses.
*/
sg_set_page(sg, NULL, PAGE_SIZE, 0);
- sg_dma_address(sg) = in[offset + src_idx];
+ sg_dma_address(sg) = sg_dma_address(src) + n*PAGE_SIZE;
sg_dma_len(sg) = PAGE_SIZE;
- sg = sg_next(sg);
- src_idx -= stride;
+ sg = __sg_next(sg);
+
+ st->nents++;
}
}
@@ -3074,62 +3079,30 @@ static noinline struct sg_table *
intel_rotate_pages(struct intel_rotation_info *rot_info,
struct drm_i915_gem_object *obj)
{
- const unsigned long n_pages = obj->base.size / PAGE_SIZE;
- unsigned int size = intel_rotation_info_size(rot_info);
- struct sgt_iter sgt_iter;
- dma_addr_t dma_addr;
- unsigned long i;
- dma_addr_t *page_addr_list;
- struct sg_table *st;
+ const unsigned int size = intel_rotation_info_size(rot_info);
struct scatterlist *sg;
+ struct sg_table *st;
+ unsigned long i;
int ret = -ENOMEM;
- /* Allocate a temporary list of source pages for random access. */
- page_addr_list = drm_malloc_gfp(n_pages,
- sizeof(dma_addr_t),
- GFP_TEMPORARY);
- if (!page_addr_list)
- return ERR_PTR(ret);
-
- /* Allocate target SG list. */
st = kmalloc(sizeof(*st), GFP_KERNEL);
if (!st)
- goto err_st_alloc;
+ goto err;
ret = sg_alloc_table(st, size, GFP_KERNEL);
if (ret)
- goto err_sg_alloc;
-
- /* Populate source page list from the object. */
- i = 0;
- for_each_sgt_dma(dma_addr, sgt_iter, obj->mm.pages)
- page_addr_list[i++] = dma_addr;
+ goto err;
- GEM_BUG_ON(i != n_pages);
st->nents = 0;
sg = st->sgl;
-
- for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++) {
- sg = rotate_pages(page_addr_list, rot_info->plane[i].offset,
- rot_info->plane[i].width, rot_info->plane[i].height,
- rot_info->plane[i].stride, st, sg);
- }
-
- DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
- obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
-
- drm_free_large(page_addr_list);
+ for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++)
+ sg = rotate_pages(obj, &rot_info->plane[i], st, sg);
+ GEM_BUG_ON(st->nents != size);
return st;
-err_sg_alloc:
+err:
kfree(st);
-err_st_alloc:
- drm_free_large(page_addr_list);
-
- DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
- obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
-
return ERR_PTR(ret);
}
--
2.11.0
_______________________________________________
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
* [CI 2/6] drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
@ 2017-02-17 15:13 ` Chris Wilson
2017-02-17 15:13 ` [CI 3/6] drm/i915: Break i915_spin_request() if we see an interrupt Chris Wilson
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:13 UTC (permalink / raw)
To: intel-gfx
When the timer expires for checking on interrupt processing, check to
see if any interrupts arrived within the last time period. If real
interrupts are still being delivered, we can be reassured that we
haven't missed the final interrupt as the waiter will still be woken.
Only once all activity ceases, do we have to worry about the waiter
never being woken and so need to install a timer to kick the waiter for
a slow arrival of a seqno.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 1 +
drivers/gpu/drm/i915/intel_breadcrumbs.c | 23 ++++++++++-------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 91be31617e78..57fa1bf78a85 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1033,6 +1033,7 @@ static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
static void notify_ring(struct intel_engine_cs *engine)
{
+ atomic_inc(&engine->irq_count);
set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted);
if (intel_engine_wakeup(engine))
trace_i915_gem_request_notify(engine);
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 74cb7b91b5db..d229a555db25 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -26,6 +26,11 @@
#include "i915_drv.h"
+static unsigned long wait_timeout(void)
+{
+ return round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES);
+}
+
static void intel_breadcrumbs_hangcheck(unsigned long data)
{
struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
@@ -34,8 +39,9 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
if (!b->irq_enabled)
return;
- if (time_before(jiffies, b->timeout)) {
- mod_timer(&b->hangcheck, b->timeout);
+ if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) {
+ b->hangcheck_interrupts = atomic_read(&engine->irq_count);
+ mod_timer(&b->hangcheck, wait_timeout());
return;
}
@@ -55,11 +61,6 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
i915_queue_hangcheck(engine->i915);
}
-static unsigned long wait_timeout(void)
-{
- return round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES);
-}
-
static void intel_breadcrumbs_fake_irq(unsigned long data)
{
struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
@@ -129,6 +130,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
/* No interrupts? Kick the waiter every jiffie! */
if (intel_irqs_enabled(i915)) {
+ b->hangcheck_interrupts = 0;
if (!test_bit(engine->id, &i915->gpu_error.test_irq_rings))
irq_enable(engine);
b->irq_enabled = true;
@@ -140,8 +142,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
i915_queue_hangcheck(i915);
} else {
/* Ensure we never sleep indefinitely */
- GEM_BUG_ON(!time_after(b->timeout, jiffies));
- mod_timer(&b->hangcheck, b->timeout);
+ mod_timer(&b->hangcheck, wait_timeout());
}
}
@@ -258,7 +259,6 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
GEM_BUG_ON(!next && !first);
if (next && next != &wait->node) {
GEM_BUG_ON(first);
- b->timeout = wait_timeout();
b->first_wait = to_wait(next);
rcu_assign_pointer(b->irq_seqno_bh, b->first_wait->tsk);
/* As there is a delay between reading the current
@@ -286,7 +286,6 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
if (first) {
GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
- b->timeout = wait_timeout();
b->first_wait = wait;
rcu_assign_pointer(b->irq_seqno_bh, wait->tsk);
/* After assigning ourselves as the new bottom-half, we must
@@ -396,7 +395,6 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
* the interrupt, or if we have to handle an
* exception rather than a seqno completion.
*/
- b->timeout = wait_timeout();
b->first_wait = to_wait(next);
rcu_assign_pointer(b->irq_seqno_bh, b->first_wait->tsk);
if (b->first_wait->seqno != wait->seqno)
@@ -627,7 +625,6 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
__intel_breadcrumbs_disable_irq(b);
if (intel_engine_has_waiter(engine)) {
- b->timeout = wait_timeout();
__intel_breadcrumbs_enable_irq(b);
if (test_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted))
wake_up_process(b->first_wait->tsk);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 4ca5308e12bb..4091c97be6ec 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -213,6 +213,7 @@ struct intel_engine_cs {
struct intel_render_state *render_state;
+ atomic_t irq_count;
unsigned long irq_posted;
#define ENGINE_IRQ_BREADCRUMB 0
#define ENGINE_IRQ_EXECLIST 1
@@ -245,7 +246,7 @@ struct intel_engine_cs {
struct timer_list fake_irq; /* used after a missed interrupt */
struct timer_list hangcheck; /* detect missed interrupts */
- unsigned long timeout;
+ unsigned int hangcheck_interrupts;
bool irq_enabled : 1;
bool rpm_wakelock : 1;
--
2.11.0
_______________________________________________
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
* [CI 3/6] drm/i915: Break i915_spin_request() if we see an interrupt
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
2017-02-17 15:13 ` [CI 2/6] drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease Chris Wilson
@ 2017-02-17 15:13 ` Chris Wilson
2017-02-17 15:13 ` [CI 4/6] drm/i915: Defer declaration of missed-interrupt until the waiter is asleep Chris Wilson
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:13 UTC (permalink / raw)
To: intel-gfx
If an interrupt has been posted, and we were spinning on the active
seqno waiting for it to advance but it did not, then we can expect that
it will not see its advance in the immediate future and should call into
the irq-seqno barrier. We can stop spinning at this point, and leave the
difficulty of handling the coherency to the caller.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_request.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2f6cfa47dc61..a5fac40d2a4f 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -963,7 +963,8 @@ static bool busywait_stop(unsigned long timeout, unsigned int cpu)
bool __i915_spin_request(const struct drm_i915_gem_request *req,
int state, unsigned long timeout_us)
{
- unsigned int cpu;
+ struct intel_engine_cs *engine = req->engine;
+ unsigned int irq, cpu;
/* When waiting for high frequency requests, e.g. during synchronous
* rendering split between the CPU and GPU, the finite amount of time
@@ -975,11 +976,20 @@ bool __i915_spin_request(const struct drm_i915_gem_request *req,
* takes to sleep on a request, on the order of a microsecond.
*/
+ irq = atomic_read(&engine->irq_count);
timeout_us += local_clock_us(&cpu);
do {
if (__i915_gem_request_completed(req))
return true;
+ /* Seqno are meant to be ordered *before* the interrupt. If
+ * we see an interrupt without a corresponding seqno advance,
+ * assume we won't see one in the near future but require
+ * the engine->seqno_barrier() to fixup coherency.
+ */
+ if (atomic_read(&engine->irq_count) != irq)
+ break;
+
if (signal_pending_state(state, current))
break;
--
2.11.0
_______________________________________________
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
* [CI 4/6] drm/i915: Defer declaration of missed-interrupt until the waiter is asleep
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
2017-02-17 15:13 ` [CI 2/6] drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease Chris Wilson
2017-02-17 15:13 ` [CI 3/6] drm/i915: Break i915_spin_request() if we see an interrupt Chris Wilson
@ 2017-02-17 15:13 ` Chris Wilson
2017-02-17 15:13 ` [CI 5/6] drm/i915: Only start with the fake-irq timer if interrupts are dead Chris Wilson
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:13 UTC (permalink / raw)
To: intel-gfx
If the waiter was currently running, assume it hasn't had a chance
to process the pending interupt (e.g, low priority task on a loaded
system) and wait until it sleeps before declaring a missed interrupt.
References: https://bugs.freedesktop.org/show_bug.cgi?id=99816
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index d229a555db25..83a8b67d6427 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -45,6 +45,15 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
return;
}
+ /* If the waiter was currently running, assume it hasn't had a chance
+ * to process the pending interupt (e.g, low priority task on a loaded
+ * system) and wait until it sleeps before declaring a missed interrupt.
+ */
+ if (!intel_engine_wakeup(engine)) {
+ mod_timer(&b->hangcheck, wait_timeout());
+ return;
+ }
+
DRM_DEBUG("Hangcheck timer elapsed... %s idle\n", engine->name);
set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
--
2.11.0
_______________________________________________
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
* [CI 5/6] drm/i915: Only start with the fake-irq timer if interrupts are dead
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
` (2 preceding siblings ...)
2017-02-17 15:13 ` [CI 4/6] drm/i915: Defer declaration of missed-interrupt until the waiter is asleep Chris Wilson
@ 2017-02-17 15:13 ` Chris Wilson
2017-02-17 15:13 ` [CI 6/6] drm/i915: Remove completed fences after a wait Chris Wilson
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:13 UTC (permalink / raw)
To: intel-gfx
As a backup to waiting on a user-interrupt from the GPU, we use a heavy
and frequent timer to wake up the waiting process should we detect an
inconsistency whilst waiting. After seeing a "missed interrupt", the
next time we wait, we restart the heavy timer. This patch is more
reluctant to restart the timer and will only do so if we have not see any
interrupts since when we started the fake irq timer. If we are seeing
interrupts, then the waiters are being woken normally and we had an
incoherency that caused to miss last time - that is unlikely to reoccur
and so taking the risk of stalling again seems pragmatic.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 83a8b67d6427..1719f9da13b8 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -107,6 +107,23 @@ static void irq_disable(struct intel_engine_cs *engine)
spin_unlock(&engine->i915->irq_lock);
}
+static bool use_fake_irq(const struct intel_breadcrumbs *b)
+{
+ const struct intel_engine_cs *engine =
+ container_of(b, struct intel_engine_cs, breadcrumbs);
+
+ if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings))
+ return false;
+
+ /* Only start with the heavy weight fake irq timer if we have not
+ * seen any interrupts since enabling it the first time. If the
+ * interrupts are still arriving, it means we made a mistake in our
+ * engine->seqno_barrier(), a timing error that should be transient
+ * and unlikely to reoccur.
+ */
+ return atomic_read(&engine->irq_count) == b->hangcheck_interrupts;
+}
+
static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
{
struct intel_engine_cs *engine =
@@ -145,8 +162,7 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
b->irq_enabled = true;
}
- if (!b->irq_enabled ||
- test_bit(engine->id, &i915->gpu_error.missed_irq_rings)) {
+ if (!b->irq_enabled || use_fake_irq(b)) {
mod_timer(&b->fake_irq, jiffies + 1);
i915_queue_hangcheck(i915);
} else {
--
2.11.0
_______________________________________________
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
* [CI 6/6] drm/i915: Remove completed fences after a wait
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
` (3 preceding siblings ...)
2017-02-17 15:13 ` [CI 5/6] drm/i915: Only start with the fake-irq timer if interrupts are dead Chris Wilson
@ 2017-02-17 15:13 ` Chris Wilson
2017-02-17 16:28 ` [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Tvrtko Ursulin
2017-02-17 18:52 ` ✓ Fi.CI.BAT: success for series starting with [CI,1/6] " Patchwork
6 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 15:13 UTC (permalink / raw)
To: intel-gfx
If we wait upon the full (i.e. all shared fences, or upon an exclusive fence)
reservation object successfully, we know that all fences beneath it have
been signaled, so long as no new fences were added whilst we slept. If the
reservation_object remains the same, as detected by its seqcount, we can
then reap all the fences upon completion.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5f7b8c88eb7e..d93032875f28 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -427,7 +427,9 @@ i915_gem_object_wait_reservation(struct reservation_object *resv,
long timeout,
struct intel_rps_client *rps)
{
+ unsigned int seq = __read_seqcount_begin(&resv->seq);
struct dma_fence *excl;
+ bool prune_fences = false;
if (flags & I915_WAIT_ALL) {
struct dma_fence **shared;
@@ -452,15 +454,26 @@ i915_gem_object_wait_reservation(struct reservation_object *resv,
for (; i < count; i++)
dma_fence_put(shared[i]);
kfree(shared);
+
+ prune_fences = count && timeout >= 0;
} else {
excl = reservation_object_get_excl_rcu(resv);
}
- if (excl && timeout >= 0)
+ if (excl && timeout >= 0) {
timeout = i915_gem_object_wait_fence(excl, flags, timeout, rps);
+ prune_fences = timeout >= 0;
+ }
dma_fence_put(excl);
+ if (prune_fences && !__read_seqcount_retry(&resv->seq, seq)) {
+ reservation_object_lock(resv, NULL);
+ if (!__read_seqcount_retry(&resv->seq, seq))
+ reservation_object_add_excl_fence(resv, NULL);
+ reservation_object_unlock(resv);
+ }
+
return timeout;
}
--
2.11.0
_______________________________________________
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: [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
` (4 preceding siblings ...)
2017-02-17 15:13 ` [CI 6/6] drm/i915: Remove completed fences after a wait Chris Wilson
@ 2017-02-17 16:28 ` Tvrtko Ursulin
2017-02-17 16:37 ` Chris Wilson
2017-02-17 18:52 ` ✓ Fi.CI.BAT: success for series starting with [CI,1/6] " Patchwork
6 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2017-02-17 16:28 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 17/02/2017 15:12, Chris Wilson wrote:
> The object already stores (computed on the fly) the index to dma address
> so use it instead of reallocating a large temporary array every time we
> bind a rotated framebuffer.
On the other hand how big is the radix tree for a large framebuffer? I
remember those nodes were quite chunky and will hang around for the
lifetime of the object. While the above mentioned large temporary array
needs to be allocated only if rotated VMAs have been discarded due GGTT
pressure, no?
On the other other hand maybe the radix tree won't be so big in the
typical case, due sg entry coalescing, but it will hang around for much
longer.
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 77 ++++++++++++-------------------------
> 1 file changed, 25 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 47a38272f54c..848dbb926fd1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3043,27 +3043,32 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
> }
>
> static struct scatterlist *
> -rotate_pages(const dma_addr_t *in, unsigned int offset,
> - unsigned int width, unsigned int height,
> - unsigned int stride,
> +rotate_pages(struct drm_i915_gem_object *obj,
> + const struct intel_rotation_plane_info *p,
> struct sg_table *st, struct scatterlist *sg)
> {
> unsigned int column, row;
> - unsigned int src_idx;
>
> - for (column = 0; column < width; column++) {
> - src_idx = stride * (height - 1) + column;
> - for (row = 0; row < height; row++) {
> - st->nents++;
> + for (column = 0; column < p->width; column++) {
> + unsigned long src_idx =
> + p->stride * (p->height - 1) + column + p->offset;
> + for (row = 0; row < p->height; row++) {
> + struct scatterlist *src;
> + unsigned int n;
> +
> + src = i915_gem_object_get_sg(obj, src_idx, &n);
> + src_idx -= p->stride;
> +
> /* We don't need the pages, but need to initialize
> * the entries so the sg list can be happily traversed.
> * The only thing we need are DMA addresses.
> */
> sg_set_page(sg, NULL, PAGE_SIZE, 0);
> - sg_dma_address(sg) = in[offset + src_idx];
> + sg_dma_address(sg) = sg_dma_address(src) + n*PAGE_SIZE;
> sg_dma_len(sg) = PAGE_SIZE;
> - sg = sg_next(sg);
> - src_idx -= stride;
> + sg = __sg_next(sg);
> +
> + st->nents++;
> }
> }
>
> @@ -3074,62 +3079,30 @@ static noinline struct sg_table *
> intel_rotate_pages(struct intel_rotation_info *rot_info,
> struct drm_i915_gem_object *obj)
> {
> - const unsigned long n_pages = obj->base.size / PAGE_SIZE;
> - unsigned int size = intel_rotation_info_size(rot_info);
> - struct sgt_iter sgt_iter;
> - dma_addr_t dma_addr;
> - unsigned long i;
> - dma_addr_t *page_addr_list;
> - struct sg_table *st;
> + const unsigned int size = intel_rotation_info_size(rot_info);
> struct scatterlist *sg;
> + struct sg_table *st;
> + unsigned long i;
> int ret = -ENOMEM;
>
> - /* Allocate a temporary list of source pages for random access. */
> - page_addr_list = drm_malloc_gfp(n_pages,
> - sizeof(dma_addr_t),
> - GFP_TEMPORARY);
> - if (!page_addr_list)
> - return ERR_PTR(ret);
> -
> - /* Allocate target SG list. */
> st = kmalloc(sizeof(*st), GFP_KERNEL);
> if (!st)
> - goto err_st_alloc;
> + goto err;
>
> ret = sg_alloc_table(st, size, GFP_KERNEL);
> if (ret)
> - goto err_sg_alloc;
> -
> - /* Populate source page list from the object. */
> - i = 0;
> - for_each_sgt_dma(dma_addr, sgt_iter, obj->mm.pages)
> - page_addr_list[i++] = dma_addr;
> + goto err;
>
> - GEM_BUG_ON(i != n_pages);
> st->nents = 0;
> sg = st->sgl;
> -
> - for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++) {
> - sg = rotate_pages(page_addr_list, rot_info->plane[i].offset,
> - rot_info->plane[i].width, rot_info->plane[i].height,
> - rot_info->plane[i].stride, st, sg);
> - }
> -
> - DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
> - obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
Hm given how chatty KMS log level is this one wasn't that harmful but
OK. Use to save me looking in debugfs/i915_gem_framebuffer and eyeball
the VMA list. Granted that is much more manageable now after you added
the human readable output there.
> -
> - drm_free_large(page_addr_list);
> + for (i = 0 ; i < ARRAY_SIZE(rot_info->plane); i++)
> + sg = rotate_pages(obj, &rot_info->plane[i], st, sg);
> + GEM_BUG_ON(st->nents != size);
>
> return st;
>
> -err_sg_alloc:
> +err:
> kfree(st);
> -err_st_alloc:
> - drm_free_large(page_addr_list);
> -
> - DRM_DEBUG_KMS("Failed to create rotated mapping for object size %zu! (%ux%u tiles, %u pages)\n",
> - obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
> -
> return ERR_PTR(ret);
> }
>
>
Code looks fine. But I would like to think about numbers some more.
Regards,
Tvrtko
_______________________________________________
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: [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
2017-02-17 16:28 ` [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Tvrtko Ursulin
@ 2017-02-17 16:37 ` Chris Wilson
2017-02-20 10:36 ` Tvrtko Ursulin
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-02-17 16:37 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Fri, Feb 17, 2017 at 04:28:13PM +0000, Tvrtko Ursulin wrote:
>
> On 17/02/2017 15:12, Chris Wilson wrote:
> >The object already stores (computed on the fly) the index to dma address
> >so use it instead of reallocating a large temporary array every time we
> >bind a rotated framebuffer.
>
> On the other hand how big is the radix tree for a large framebuffer?
> I remember those nodes were quite chunky and will hang around for
> the lifetime of the object. While the above mentioned large
> temporary array needs to be allocated only if rotated VMAs have been
> discarded due GGTT pressure, no?
>
> On the other other hand maybe the radix tree won't be so big in the
> typical case, due sg entry coalescing, but it will hang around for
> much longer.
Also don't forget that we use the radixtree for mmaps, partials, single
page lookups. In all likelihood it already exists, and it doesn't hang
around forever.
> >- DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
> >- obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
>
> Hm given how chatty KMS log level is this one wasn't that harmful
> but OK. Use to save me looking in debugfs/i915_gem_framebuffer and
> eyeball the VMA list. Granted that is much more manageable now after
> you added the human readable output there.
It's just the odd one out. If it is useful here, it presumably has some
use on the other branches - and do we want it at page allocation time or
vma creation. And I don't think we really want one at vma create, so I'd
prefer to improve the debugfs (or other) probe.
-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 series starting with [CI,1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
` (5 preceding siblings ...)
2017-02-17 16:28 ` [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Tvrtko Ursulin
@ 2017-02-17 18:52 ` Patchwork
6 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-02-17 18:52 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
URL : https://patchwork.freedesktop.org/series/19852/
State : success
== Summary ==
Series 19852v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/19852/revisions/1/mbox/
fi-bdw-5557u total:252 pass:241 dwarn:0 dfail:0 fail:0 skip:11
fi-bsw-n3050 total:252 pass:213 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-j4205 total:252 pass:233 dwarn:0 dfail:0 fail:0 skip:19
fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-j1900 total:252 pass:225 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:252 pass:221 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:252 pass:236 dwarn:0 dfail:0 fail:0 skip:16
fi-hsw-4770r total:252 pass:236 dwarn:0 dfail:0 fail:0 skip:16
fi-ilk-650 total:252 pass:202 dwarn:0 dfail:0 fail:0 skip:50
fi-ivb-3520m total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-ivb-3770 total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-kbl-7500u total:252 pass:234 dwarn:0 dfail:0 fail:0 skip:18
fi-skl-6260u total:252 pass:242 dwarn:0 dfail:0 fail:0 skip:10
fi-skl-6700hq total:252 pass:235 dwarn:0 dfail:0 fail:0 skip:17
fi-skl-6700k total:252 pass:230 dwarn:4 dfail:0 fail:0 skip:18
fi-skl-6770hq total:252 pass:242 dwarn:0 dfail:0 fail:0 skip:10
fi-snb-2520m total:252 pass:224 dwarn:0 dfail:0 fail:0 skip:28
fi-snb-2600 total:252 pass:223 dwarn:0 dfail:0 fail:0 skip:29
02022d17a5787709617b7897de3906970e2b0721 drm-tip: 2017y-02m-17d-15h-15m-45s UTC integration manifest
ceced16 drm/i915: Remove completed fences after a wait
91f2b88 drm/i915: Only start with the fake-irq timer if interrupts are dead
a143470 drm/i915: Defer declaration of missed-interrupt until the waiter is asleep
adb3285 drm/i915: Break i915_spin_request() if we see an interrupt
c38a802 drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease
b948281 drm/i915: Remove temporary allocation of dma addresses when rotating
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3885/
_______________________________________________
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: [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
2017-02-17 16:37 ` Chris Wilson
@ 2017-02-20 10:36 ` Tvrtko Ursulin
2017-02-20 10:46 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2017-02-20 10:36 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 17/02/2017 16:37, Chris Wilson wrote:
> On Fri, Feb 17, 2017 at 04:28:13PM +0000, Tvrtko Ursulin wrote:
>>
>> On 17/02/2017 15:12, Chris Wilson wrote:
>>> The object already stores (computed on the fly) the index to dma address
>>> so use it instead of reallocating a large temporary array every time we
>>> bind a rotated framebuffer.
>>
>> On the other hand how big is the radix tree for a large framebuffer?
>> I remember those nodes were quite chunky and will hang around for
>> the lifetime of the object. While the above mentioned large
>> temporary array needs to be allocated only if rotated VMAs have been
>> discarded due GGTT pressure, no?
>>
>> On the other other hand maybe the radix tree won't be so big in the
>> typical case, due sg entry coalescing, but it will hang around for
>> much longer.
>
> Also don't forget that we use the radixtree for mmaps, partials, single
> page lookups. In all likelihood it already exists, and it doesn't hang
> around forever.
Yeah, but for a typical framebuffer none of this are actually used to
trigger creating the radix tree.
For a standard 1920x1080x32 fb, temporary allocation for rotation is
only ~16KiB so don't see that it can cause any problems even with 4k
screens.
Struct radix_tree_node is much chunkier so how many of those will a
typical framebuffer need and how much memory in total would it tie up?
It would only be dropped once we drop the backing store, so for
framebuffers basically never.
>>> - DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
>>> - obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
>>
>> Hm given how chatty KMS log level is this one wasn't that harmful
>> but OK. Use to save me looking in debugfs/i915_gem_framebuffer and
>> eyeball the VMA list. Granted that is much more manageable now after
>> you added the human readable output there.
>
> It's just the odd one out. If it is useful here, it presumably has some
> use on the other branches - and do we want it at page allocation time or
> vma creation. And I don't think we really want one at vma create, so I'd
> prefer to improve the debugfs (or other) probe.
I agree that debugfs output is sufficent nowadays.
Regards,
Tvrtko
_______________________________________________
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: [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating
2017-02-20 10:36 ` Tvrtko Ursulin
@ 2017-02-20 10:46 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-20 10:46 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
On Mon, Feb 20, 2017 at 10:36:33AM +0000, Tvrtko Ursulin wrote:
>
> On 17/02/2017 16:37, Chris Wilson wrote:
> >On Fri, Feb 17, 2017 at 04:28:13PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 17/02/2017 15:12, Chris Wilson wrote:
> >>>The object already stores (computed on the fly) the index to dma address
> >>>so use it instead of reallocating a large temporary array every time we
> >>>bind a rotated framebuffer.
> >>
> >>On the other hand how big is the radix tree for a large framebuffer?
> >>I remember those nodes were quite chunky and will hang around for
> >>the lifetime of the object. While the above mentioned large
> >>temporary array needs to be allocated only if rotated VMAs have been
> >>discarded due GGTT pressure, no?
> >>
> >>On the other other hand maybe the radix tree won't be so big in the
> >>typical case, due sg entry coalescing, but it will hang around for
> >>much longer.
> >
> >Also don't forget that we use the radixtree for mmaps, partials, single
> >page lookups. In all likelihood it already exists, and it doesn't hang
> >around forever.
>
> Yeah, but for a typical framebuffer none of this are actually used
> to trigger creating the radix tree.
>
> For a standard 1920x1080x32 fb, temporary allocation for rotation is
> only ~16KiB so don't see that it can cause any problems even with 4k
> screens.
>
> Struct radix_tree_node is much chunkier so how many of those will a
> typical framebuffer need and how much memory in total would it tie
> up? It would only be dropped once we drop the backing store, so for
> framebuffers basically never.
Typically? We're talking about how often do people do GTT mmaps even
though we keep telling them not to?
-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
end of thread, other threads:[~2017-02-20 10:46 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-17 15:12 [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Chris Wilson
2017-02-17 15:13 ` [CI 2/6] drm/i915: Postpone fake breadcrumb interrupt until real interrupts cease Chris Wilson
2017-02-17 15:13 ` [CI 3/6] drm/i915: Break i915_spin_request() if we see an interrupt Chris Wilson
2017-02-17 15:13 ` [CI 4/6] drm/i915: Defer declaration of missed-interrupt until the waiter is asleep Chris Wilson
2017-02-17 15:13 ` [CI 5/6] drm/i915: Only start with the fake-irq timer if interrupts are dead Chris Wilson
2017-02-17 15:13 ` [CI 6/6] drm/i915: Remove completed fences after a wait Chris Wilson
2017-02-17 16:28 ` [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating Tvrtko Ursulin
2017-02-17 16:37 ` Chris Wilson
2017-02-20 10:36 ` Tvrtko Ursulin
2017-02-20 10:46 ` Chris Wilson
2017-02-17 18:52 ` ✓ Fi.CI.BAT: success for series starting with [CI,1/6] " 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.