Op 12-08-2020 om 21:53 schreef Thomas Hellström (Intel):
>
> On 8/10/20 12:30 PM, Maarten Lankhorst wrote:
>> We have the ordering of timeline->mutex vs resv_lock wrong,
>> convert the i915_pin_vma and intel_context_pin as well to
>> future-proof this.
>>
>> We may need to do future changes to do this more transaction-like,
>> and only get down to a single i915_gem_ww_ctx, but for now this
>> should work.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_perf.c | 57 +++++++++++++++++++++++---------
>> 1 file changed, 42 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index c6f6370283cf..e94976976571 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1195,24 +1195,39 @@ static struct intel_context *oa_pin_context(struct i915_perf_stream *stream)
>> struct i915_gem_engines_iter it;
>> struct i915_gem_context *ctx = stream->ctx;
>> struct intel_context *ce;
>> - int err;
>> + struct i915_gem_ww_ctx ww;
>> + int err = -ENODEV;
>> for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
>> if (ce->engine != stream->engine) /* first match! */
>> continue;
>> - /*
>> - * As the ID is the gtt offset of the context's vma we
>> - * pin the vma to ensure the ID remains fixed.
>> - */
>> - err = intel_context_pin(ce);
>> - if (err == 0) {
>> - stream->pinned_ctx = ce;
>> - break;
>> - }
>> + err = 0;
>> + break;
>> }
>> i915_gem_context_unlock_engines(ctx);
>> + if (err)
>> + return ERR_PTR(err);
>> +
>> + i915_gem_ww_ctx_init(&ww, true);
>> +retry:
>> + /*
>> + * As the ID is the gtt offset of the context's vma we
>> + * pin the vma to ensure the ID remains fixed.
>> + */
>> + err = intel_context_pin_ww(ce, &ww);
>> + if (err == -EDEADLK) {
>> + err = i915_gem_ww_ctx_backoff(&ww);
>> + if (!err)
>> + goto retry;
>> + }
>> + i915_gem_ww_ctx_fini(&ww);
>> +
>
> Hmm. Didn't we keep an intel_context_pin() that does exactly the above without recoding the whole ww transaction? Or do you plan to remove that?
>
> With that taken into account,
>
> Reviewed-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxx>
>
>
Yeah, I want to remove that eventually, might need to change i915_perf even more to fully do this. Thanks for reviewing.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
