Re: [PATCH 04/23] drm/i915: Push the ring creation flags to the backend

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 26/07/2019 09:43, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-24 12:11:54)
>>
>> On 23/07/2019 19:38, Chris Wilson wrote:
>>> Push the ring creation flags from the outer GEM context to the inner
>>> intel_cotnext to avoid an unsightly back-reference from inside the
>>> backend.
>>
>> Sorry I find this quite ugly. Passing in integers in pointers filed and
>> casting back and forth.
> 
> But better than a union, since once the intel_context is active, the
> ring is always a ring.

Unless it is u64 size. I am not buying it. :)

>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 6d3911469801..e237bcecfa1f 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -328,10 +328,14 @@ static void print_context_stats(struct seq_file *m,
>>>    
>>>                for_each_gem_engine(ce,
>>>                                    i915_gem_context_lock_engines(ctx), it) {
>>> -                     if (ce->state)
>>> -                             per_file_stats(0, ce->state->obj, &kstats);
>>> -                     if (ce->ring)
>>> +                     intel_context_lock_pinned(ce);
>>> +                     if (intel_context_is_pinned(ce)) {
>>
>> And these hunks do not seem to belong to this patch.
> 
> Then you are mistaken. The bug is older, but it gets triggered by this
> patch.

Gets triggered or gets fixed? Perhaps commit message needs improving 
since it talks about avoiding an unsightly back-reference (and I argue 
ce->ring = u64 size is at least equally unsightly), and not fixing any bugs.

Regards,

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