Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

I think Chris' comments show this isn't actually tested.
Can you please write at least one test case for igt ?

We have a pretty long list of patches that need to land (still need 
review on some patches).
I would recommend you base you patches on this :

https://github.com/djdeath/intel-gpu-tools/tree/wip/djdeath/oa-next

Once your tests are passing, I'll put them in that branch.

Thanks!

On 21/07/17 12:04, Chris Wilson wrote:
> Quoting Zhenyu Wang (2017-07-19 06:39:48)
>> +static struct i915_gem_context *
>> +lookup_context_hw_id(struct drm_i915_private *dev_priv, unsigned int hw_id)
>> +{
>> +       struct i915_gem_context *ctx;
>> +       int ret;
>> +
>> +       ret = i915_mutex_lock_interruptible(&dev_priv->drm);
>> +       if (ret)
>> +               return ERR_PTR(ret);
>> +
>> +       list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
>> +               if (!i915_gem_context_is_default(ctx))
>> +                       continue;
> This is still a massive what? Why ban normal contexts? Why are you not
> banning the kernel context?
>
>> +
>> +               if (ctx->hw_id == hw_id) {
>> +                       ret = 1;
>> +                       i915_gem_context_get(ctx);
>> +                       break;
> Bad news, your reference counting is still broken. You actually need an
> i915_gem_context_get_rcu() variant for pulling from this list.
> -Chris
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx