| Message ID | 20210915114153.951670-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/debugfs: Do not report currently active engine when describing objects | expand |
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. 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. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > 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 >
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 >> >
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)