Quoting Lionel Landwerlin (2020-04-30 14:55:35)
> @@ -1382,6 +1446,12 @@ static void i915_oa_stream_destroy(struct 
> i915_perf_stream *stream)
>  
>         BUG_ON(stream != perf->exclusive_stream);
>  
> +       err = intel_context_pin(stream->config_context);
> +       if (err) {
> +               drm_err(&perf->i915->drm,
> +                       "Error pinning i915-perf context\n");
> +       }

Reading through, this is the last thing that leapt out at me.

Taking an error here would be nasty, and we do allow the pin to be
interrupted (which would be a surprise for most people who call
close()).

The only suggestion I can make, is that the stream always keeps it
pinned.

>From the robustness point of view, it's better if we break
stream->config_context than if we break stream->engine->kernel_context,
but equally pinning another context image [about 80k in total] just for
safety? Probably worth it. [If we break the kernel_context, we cannot do
hang detection or pm/idle, the driver comes grinding to a halt.]
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to