On 27/06/2019 15:53, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-06-27 13:32:13)
On 27/06/2019 12:45, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-06-27 09:00:42)
+ /*
+ * If the config hasn't changed, skip reconfiguring the HW (this is
+ * subject to a delay we want to avoid has much as possible).
+ */
+ if (eb->oa_config == eb->i915->perf.oa.exclusive_stream->oa_config)
+ return 0;
+
+ oa_vma = i915_vma_instance(eb->oa_bo,
+ &eb->engine->i915->ggtt.vm, NULL);
+ if (unlikely(IS_ERR(oa_vma)))
+ return PTR_ERR(oa_vma);
+
+ err = i915_vma_pin(oa_vma, 0, 0, PIN_GLOBAL);
+ if (err)
+ return err;
Ugh. We should not be pinned after creating the request. Might not
matter -- it's just reservation under its own lock that must not be
crossed with the timeline lock currently held here.
Should I move this into get_execbuf_oa_config() ?
That would be save me fretting about the lock nesting.
@@ -2651,9 +2760,23 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (unlikely(err))
goto err_unlock;
+ if (eb.extensions.flags & BIT(DRM_I915_GEM_EXECBUFFER_EXT_PERF)) {
+ if (!intel_engine_has_oa(eb.engine)) {
+ err = -ENODEV;
+ goto err_engine;
+ }
+
+ err = get_execbuf_oa_config(eb.i915,
+ eb.extensions.perf_config.perf_fd,
+ eb.extensions.perf_config.oa_config,
+ &eb.oa_config, &eb.oa_bo);
+ if (err)
+ goto err_engine;
Why is this under the struct_mutex?
Access to dev_priv->perf.oa.exclusive_stream must be under struct_mutex.
Also because we verify that the engine actually has OA support.
I could split the getting the configuration part away.
I'm about 10^W 50^W certainly less than a 100 patches away from killing
struct_mutex for execbuf...
-Chris
I'm sorry. Dealing with all this OA stuff is underwhelming.
I think an engine lock would be enough if that's not too bad for you.
-Lionel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
