On 01/07/2019 18:09, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-07-01 12:34:29)struct i915_oa_config { + struct drm_i915_private *i915; + char uuid[UUID_STRING_LEN + 1]; int id;@@ -1110,6 +1112,10 @@ struct i915_oa_config {struct attribute *attrs[2]; struct device_attribute sysfs_metric_id;+ struct drm_i915_gem_object *obj;+ + struct list_head vma_link; + atomic_t ref_count; }; -static void free_oa_config(struct drm_i915_private *dev_priv, - struct i915_oa_config *oa_config) +static void put_oa_config(struct i915_oa_config *oa_config) { + if (!atomic_dec_and_test(&oa_config->ref_count)) + return;I strongly advise that ref_count be replaced by struct kref, just so that we get the benefit of debugging. put_oa_config -> kref_put(&oa_config->ref, free_oa_config) (free_oa_config takes kref as its arg and uses container_of())
This is done in "drm/i915: add a new perf configuration execbuf parameter" I'll factor it in this commit.
+int i915_perf_get_oa_config(struct drm_i915_private *i915, + int metrics_set, + struct i915_oa_config **out_config, + struct drm_i915_gem_object **out_obj) +{ + int ret = 0; + struct i915_oa_config *oa_config; + + if (!i915->perf.initialized) + return -ENODEV; + + ret = mutex_lock_interruptible(&i915->perf.metrics_lock); if (ret) return ret;- *out_config = idr_find(&dev_priv->perf.metrics_idr, metrics_set);- if (!*out_config) - ret = -EINVAL; - else - atomic_inc(&(*out_config)->ref_count); + if (metrics_set == 1) { + oa_config = &i915->perf.oa.test_config; + } else { + oa_config = idr_find(&i915->perf.metrics_idr, metrics_set);Why not have the builtin[1] inside the idr?
I think it was just a way to avoid removing it from the idr through userspace calls.
-Chris
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
