[Intel-gfx] [PATCH v6 03/11] drm/i915/perf: allow for CS OA configs to be created lazily

[Chris Wilson]

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())

> +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?
-Chris