On 10/03/2020 13:17, Chris Wilson wrote:
Quoting Chris Wilson (2020-03-06 13:38:42)
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 UNINTERRUPTIBLE you might ask?
Because if you've demonstrated that by having scheduled the oa config
update that by not waiting for the change, the machine becomes unusable,
that seems like a risk not worth taking.
Just to confirm, the risk would be that the task could be interrupted
and that we would schedule another configuration request, without any
way and that would bring us back to the buggy scenario we saw.
-Lionel
Hence why the i915_request_wait() was uninterruptible and the
i915_active_wait() keeps the uninterruptible nature.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
