Re: [PATCH] drm/i915/debugfs: Do not report currently active engine when describing objects

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

On 17/09/2021 13:43, Daniel Vetter wrote:
> On Wed, Sep 15, 2021 at 12:41:53PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> It is not very useful to have code which tries to report a rapidly
>> transient state which will not report anything majority of the time,
>> especially since it is currently only used from
>> <debugfs>/i915_gem_framebuffers.
> 
> Maybe mention that this is motiveated by Christian's series to rework all
> the fence iteration stuff.

It actually isn't. It predates me noticing his series and is from the 
time I was looking at all the dma_fence_is_i915() call sites. One part 
of the rationale was fewer users the better.

> Also iirc the history here, this was for figuring out hangs. When the gpu
> is dead, the last-write stuff isn't very transient anymore (until reset).
> But we have error state capture now since forever, so this can go.

In this case say 8-9 seconds before it gets cleared so yes, still agreed 
it is not very useful.

> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks!

Regards,

Tvrtko

> 
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Acked-by: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_object.h | 17 -----------------
>>   drivers/gpu/drm/i915/i915_debugfs.c        |  5 -----
>>   2 files changed, 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>> index 48112b9d76df..3043fcbd31bd 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
>> @@ -503,23 +503,6 @@ i915_gem_object_finish_access(struct drm_i915_gem_object *obj)
>>   	i915_gem_object_unpin_pages(obj);
>>   }
>>   
>> -static inline struct intel_engine_cs *
>> -i915_gem_object_last_write_engine(struct drm_i915_gem_object *obj)
>> -{
>> -	struct intel_engine_cs *engine = NULL;
>> -	struct dma_fence *fence;
>> -
>> -	rcu_read_lock();
>> -	fence = dma_resv_get_excl_unlocked(obj->base.resv);
>> -	rcu_read_unlock();
>> -
>> -	if (fence && dma_fence_is_i915(fence) && !dma_fence_is_signaled(fence))
>> -		engine = to_request(fence)->engine;
>> -	dma_fence_put(fence);
>> -
>> -	return engine;
>> -}
>> -
>>   void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
>>   					 unsigned int cache_level);
>>   void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 04351a851586..1795af81bf41 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -139,7 +139,6 @@ void
>>   i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
>> -	struct intel_engine_cs *engine;
>>   	struct i915_vma *vma;
>>   	int pin_count = 0;
>>   
>> @@ -229,10 +228,6 @@ i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>>   		seq_printf(m, " (stolen: %08llx)", obj->stolen->start);
>>   	if (i915_gem_object_is_framebuffer(obj))
>>   		seq_printf(m, " (fb)");
>> -
>> -	engine = i915_gem_object_last_write_engine(obj);
>> -	if (engine)
>> -		seq_printf(m, " (%s)", engine->name);
>>   }
>>   
>>   static int i915_gem_object_info(struct seq_file *m, void *data)
>> -- 
>> 2.30.2
>>
>