| Message ID | 20220504183716.987793-2-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: remove unused GEM_DEBUG_DECL() and GEM_DEBUG_BUG_ON() | expand |
On 04/05/2022 19:37, Jani Nikula wrote: > Reduce the magic of what's going on in GEM_DEBUG_EXEC() by expanding it > inline and being explicit about it. It's as single use case anyway, so > the macro feels overkill. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++- > drivers/gpu/drm/i915/i915_gem.h | 2 -- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c > index 40ffcb94e379..15ec64d881c4 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ring.c > +++ b/drivers/gpu/drm/i915/gt/intel_ring.c > @@ -299,7 +299,8 @@ u32 *intel_ring_begin(struct i915_request *rq, unsigned int num_dwords) > GEM_BUG_ON(ring->emit > ring->size - bytes); > GEM_BUG_ON(ring->space < bytes); > cs = ring->vaddr + ring->emit; > - GEM_DEBUG_EXEC(memset32(cs, POISON_INUSE, bytes / sizeof(*cs))); > + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) > + memset32(cs, POISON_INUSE, bytes / sizeof(*cs)); > ring->emit += bytes; > ring->space -= bytes; > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > index b7b257f54d2e..a2be323a4be5 100644 > --- a/drivers/gpu/drm/i915/i915_gem.h > +++ b/drivers/gpu/drm/i915/i915_gem.h > @@ -54,7 +54,6 @@ struct drm_i915_private; > } while(0) > #define GEM_WARN_ON(expr) WARN_ON(expr) > > -#define GEM_DEBUG_EXEC(expr) expr > #define GEM_DEBUG_WARN_ON(expr) GEM_WARN_ON(expr) > > #else > @@ -64,7 +63,6 @@ struct drm_i915_private; > #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) > #define GEM_WARN_ON(expr) ({ unlikely(!!(expr)); }) > > -#define GEM_DEBUG_EXEC(expr) do { } while (0) > #define GEM_DEBUG_WARN_ON(expr) ({ BUILD_BUG_ON_INVALID(expr); 0; }) > #endif > Yeah one user after time passed suggests removing is the right course of action. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On Thu, 05 May 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > On 04/05/2022 19:37, Jani Nikula wrote: >> Reduce the magic of what's going on in GEM_DEBUG_EXEC() by expanding it >> inline and being explicit about it. It's as single use case anyway, so >> the macro feels overkill. >> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++- >> drivers/gpu/drm/i915/i915_gem.h | 2 -- >> 2 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c >> index 40ffcb94e379..15ec64d881c4 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_ring.c >> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c >> @@ -299,7 +299,8 @@ u32 *intel_ring_begin(struct i915_request *rq, unsigned int num_dwords) >> GEM_BUG_ON(ring->emit > ring->size - bytes); >> GEM_BUG_ON(ring->space < bytes); >> cs = ring->vaddr + ring->emit; >> - GEM_DEBUG_EXEC(memset32(cs, POISON_INUSE, bytes / sizeof(*cs))); >> + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) >> + memset32(cs, POISON_INUSE, bytes / sizeof(*cs)); >> ring->emit += bytes; >> ring->space -= bytes; >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h >> index b7b257f54d2e..a2be323a4be5 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.h >> +++ b/drivers/gpu/drm/i915/i915_gem.h >> @@ -54,7 +54,6 @@ struct drm_i915_private; >> } while(0) >> #define GEM_WARN_ON(expr) WARN_ON(expr) >> >> -#define GEM_DEBUG_EXEC(expr) expr >> #define GEM_DEBUG_WARN_ON(expr) GEM_WARN_ON(expr) >> >> #else >> @@ -64,7 +63,6 @@ struct drm_i915_private; >> #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) >> #define GEM_WARN_ON(expr) ({ unlikely(!!(expr)); }) >> >> -#define GEM_DEBUG_EXEC(expr) do { } while (0) >> #define GEM_DEBUG_WARN_ON(expr) ({ BUILD_BUG_ON_INVALID(expr); 0; }) >> #endif >> > > Yeah one user after time passed suggests removing is the right course of > action. > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Thanks, pushed the first two to din. BR, Jani. > > Regards, > > Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c index 40ffcb94e379..15ec64d881c4 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring.c +++ b/drivers/gpu/drm/i915/gt/intel_ring.c @@ -299,7 +299,8 @@ u32 *intel_ring_begin(struct i915_request *rq, unsigned int num_dwords) GEM_BUG_ON(ring->emit > ring->size - bytes); GEM_BUG_ON(ring->space < bytes); cs = ring->vaddr + ring->emit; - GEM_DEBUG_EXEC(memset32(cs, POISON_INUSE, bytes / sizeof(*cs))); + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) + memset32(cs, POISON_INUSE, bytes / sizeof(*cs)); ring->emit += bytes; ring->space -= bytes; diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index b7b257f54d2e..a2be323a4be5 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -54,7 +54,6 @@ struct drm_i915_private; } while(0) #define GEM_WARN_ON(expr) WARN_ON(expr) -#define GEM_DEBUG_EXEC(expr) expr #define GEM_DEBUG_WARN_ON(expr) GEM_WARN_ON(expr) #else @@ -64,7 +63,6 @@ struct drm_i915_private; #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) #define GEM_WARN_ON(expr) ({ unlikely(!!(expr)); }) -#define GEM_DEBUG_EXEC(expr) do { } while (0) #define GEM_DEBUG_WARN_ON(expr) ({ BUILD_BUG_ON_INVALID(expr); 0; }) #endif
Reduce the magic of what's going on in GEM_DEBUG_EXEC() by expanding it inline and being explicit about it. It's as single use case anyway, so the macro feels overkill. Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/gt/intel_ring.c | 3 ++- drivers/gpu/drm/i915/i915_gem.h | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-)