| Message ID | 1452503961-14837-6-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/01/16 09:16, Chris Wilson wrote: > Currently there is a #define to enable extra BUG_ON for debugging > requests and associated activities. I want to expand its use to cover > all of GEM internals (so that we can saturate the code with asserts). > We can add a Kconfig option to make it easier to enable - with the usual > caveats of not enabling unless explicitly requested. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/Kconfig.debug | 8 ++++++++ > drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ > drivers/gpu/drm/i915/i915_gem.c | 12 +++++------- > 3 files changed, 19 insertions(+), 7 deletions(-) > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug > index 1f10ee228eda..7fa6b97635e5 100644 > --- a/drivers/gpu/drm/i915/Kconfig.debug > +++ b/drivers/gpu/drm/i915/Kconfig.debug > @@ -10,3 +10,11 @@ config DRM_I915_WERROR > ---help--- > Add -Werror to the build flags for (and only for) i915.ko. > Do not enable this unless you are writing code for the i915.ko module. > + > +config DRM_I915_DEBUG_GEM > + bool "Insert extra checks into the GEM internals" > + default n > + depends on DRM_I915_WERROR This comes up as an option only if DRM_I915_WERROR is already selected? Surely it should be orthogonal to compile-time checks, with each as independent options but both restricted to EXPERT mode. So the line above should be "depends on EXPERT" not "depends on DRM_I915_WERROR"? > + ---help--- > + Enable extra sanity checks (including BUGs) that may slow the > + system down and if hit hang the machine. "hang the machine if hit". Unless you want commas round "if hit"? Otherwise looks OK. .Dave.
diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 1f10ee228eda..7fa6b97635e5 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -10,3 +10,11 @@ config DRM_I915_WERROR ---help--- Add -Werror to the build flags for (and only for) i915.ko. Do not enable this unless you are writing code for the i915.ko module. + +config DRM_I915_DEBUG_GEM + bool "Insert extra checks into the GEM internals" + default n + depends on DRM_I915_WERROR + ---help--- + Enable extra sanity checks (including BUGs) that may slow the + system down and if hit hang the machine. diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ec20814adb0c..1a6168affadd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2271,6 +2271,12 @@ struct drm_i915_gem_request { }; +#ifdef CONFIG_DRM_I915_DEBUG_GEM +#define GEM_BUG_ON(expr) BUG_ON(expr) +#else +#define GEM_BUG_ON(expr) +#endif + int i915_gem_request_alloc(struct intel_engine_cs *ring, struct intel_context *ctx, struct drm_i915_gem_request **req_out); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fd24877eb0a0..99fd6aa4dd62 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -38,8 +38,6 @@ #include <linux/pci.h> #include <linux/dma-buf.h> -#define RQ_BUG_ON(expr) - static void i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj); static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj); static void @@ -1520,7 +1518,7 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj, i915_gem_object_retire__read(obj, i); } - RQ_BUG_ON(obj->active); + GEM_BUG_ON(obj->active); } return 0; @@ -2430,8 +2428,8 @@ void i915_vma_move_to_active(struct i915_vma *vma, static void i915_gem_object_retire__write(struct drm_i915_gem_object *obj) { - RQ_BUG_ON(obj->last_write_req == NULL); - RQ_BUG_ON(!(obj->active & intel_ring_flag(obj->last_write_req->ring))); + GEM_BUG_ON(obj->last_write_req == NULL); + GEM_BUG_ON(!(obj->active & intel_ring_flag(obj->last_write_req->ring))); i915_gem_request_assign(&obj->last_write_req, NULL); intel_fb_obj_flush(obj, true, ORIGIN_CS); @@ -2442,8 +2440,8 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring) { struct i915_vma *vma; - RQ_BUG_ON(obj->last_read_req[ring] == NULL); - RQ_BUG_ON(!(obj->active & (1 << ring))); + GEM_BUG_ON(obj->last_read_req[ring] == NULL); + GEM_BUG_ON(!(obj->active & (1 << ring))); list_del_init(&obj->ring_list[ring]); i915_gem_request_assign(&obj->last_read_req[ring], NULL);
Currently there is a #define to enable extra BUG_ON for debugging requests and associated activities. I want to expand its use to cover all of GEM internals (so that we can saturate the code with asserts). We can add a Kconfig option to make it easier to enable - with the usual caveats of not enabling unless explicitly requested. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/Kconfig.debug | 8 ++++++++ drivers/gpu/drm/i915/i915_drv.h | 6 ++++++ drivers/gpu/drm/i915/i915_gem.c | 12 +++++------- 3 files changed, 19 insertions(+), 7 deletions(-)