On 06/03/2020 15:38, Chris Wilson wrote:
We wish that the scheduler emit the context modification commands prior
to enabling the oa_config, for which we must explicitly inform it of the
ordering constraints. This is especially important as we now wait for
the final oa_config setup to be completed and as this wait may be on a
distinct context to the state modifications, we need that command packet
to be always last in the queue.

We borrow the i915_active for its ability to track multiple timelines
and the last dma_fence on each; a flexible dma_resv. Keeping track of
each dma_fence is important for us so that we can efficiently schedule
the requests and reprioritise as required.

Reported-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwer...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_overlay.c  |   8 +-
  drivers/gpu/drm/i915/gt/intel_context_param.c |   2 +-
  drivers/gpu/drm/i915/i915_active.c            |   6 +-
  drivers/gpu/drm/i915/i915_active.h            |   2 +-
  drivers/gpu/drm/i915/i915_perf.c              | 154 +++++++++++-------
  drivers/gpu/drm/i915/i915_perf_types.h        |   5 +-
  drivers/gpu/drm/i915/i915_vma.h               |   2 +-
  drivers/gpu/drm/i915/selftests/i915_active.c  |   4 +-
  8 files changed, 115 insertions(+), 68 deletions(-)

...
@@ -2729,16 +2772,19 @@ static const struct i915_perf_stream_ops 
i915_oa_stream_ops = {
static int i915_perf_stream_enable_sync(struct i915_perf_stream *stream)
  {
-       struct i915_request *rq;
+       struct i915_active *active;
+       int err;
- rq = stream->perf->ops.enable_metric_set(stream);
-       if (IS_ERR(rq))
-               return PTR_ERR(rq);
+       active = i915_active_create();
+       if (!active)
+               return -ENOMEM;
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
-       i915_request_put(rq);
+       err = stream->perf->ops.enable_metric_set(stream, active);
+       if (err == 0)
+               i915_active_wait(active, TASK_UNINTERRUPTIBLE);


Why not? :


err = i915_active_wait(active, TASK_INTERRUPTIBLE);


-Lionel


- return 0;
+       i915_active_put(active);
+       return err;
  }
/**
@@ -3192,7 +3238,7 @@ static long i915_perf_config_locked(struct 
i915_perf_stream *stream,
                return -EINVAL;
if (config != stream->oa_config) {
-               struct i915_request *rq;
+               int err;
/*
                 * If OA is bound to a specific context, emit the
@@ -3203,13 +3249,11 @@ static long i915_perf_config_locked(struct 
i915_perf_stream *stream,
                 * When set globally, we use a low priority kernel context,
                 * so it will effectively take effect when idle.
                 */
-               rq = emit_oa_config(stream, config, oa_context(stream));
-               if (!IS_ERR(rq)) {
+               err = emit_oa_config(stream, config, oa_context(stream), NULL);
+               if (!err)
                        config = xchg(&stream->oa_config, config);
-                       i915_request_put(rq);
-               } else {
-                       ret = PTR_ERR(rq);
-               }
+               else
+                       ret = err;
        }
i915_oa_config_put(config);
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h 
b/drivers/gpu/drm/i915/i915_perf_types.h
index a0e22f00f6cf..5eaf874a0d25 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -21,6 +21,7 @@
struct drm_i915_private;
  struct file;
+struct i915_active;
  struct i915_gem_context;
  struct i915_perf;
  struct i915_vma;
@@ -339,8 +340,8 @@ struct i915_oa_ops {
         * counter reports being sampled. May apply system constraints such as
         * disabling EU clock gating as required.
         */
-       struct i915_request *
-               (*enable_metric_set)(struct i915_perf_stream *stream);
+       int (*enable_metric_set)(struct i915_perf_stream *stream,
+                                struct i915_active *active);
/**
         * @disable_metric_set: Remove system constraints associated with using
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index e1ced1df13e1..3baa98fa5009 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -380,7 +380,7 @@ int i915_vma_wait_for_bind(struct i915_vma *vma);
  static inline int i915_vma_sync(struct i915_vma *vma)
  {
        /* Wait for the asynchronous bindings and pending GPU reads */
-       return i915_active_wait(&vma->active);
+       return i915_active_wait(&vma->active, TASK_INTERRUPTIBLE);
  }
#endif
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c 
b/drivers/gpu/drm/i915/selftests/i915_active.c
index 68bbb1580162..7357d2130024 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -153,7 +153,7 @@ static int live_active_wait(void *arg)
        if (IS_ERR(active))
                return PTR_ERR(active);
- i915_active_wait(&active->base);
+       i915_active_wait(&active->base, TASK_UNINTERRUPTIBLE);
        if (!READ_ONCE(active->retired)) {
                struct drm_printer p = drm_err_printer(__func__);
@@ -230,7 +230,7 @@ static int live_active_barrier(void *arg)
        i915_active_release(&active->base);
if (err == 0)
-               err = i915_active_wait(&active->base);
+               err = i915_active_wait(&active->base, TASK_UNINTERRUPTIBLE);
if (err == 0 && !READ_ONCE(active->retired)) {
                pr_err("i915_active not retired after flushing barriers!\n");


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

Reply via email to