[Intel-gfx] [PATCH] drm/i915: Use BUILD_BUG if possible in the i915 WARN_ON

Chris Wilson chris at chris-wilson.co.uk
Wed Dec 10 05:53:26 PST 2014


On Wed, Dec 10, 2014 at 02:49:05PM +0100, Daniel Vetter wrote:
> Faster feedback to errors is always better. This is inspired by the
> addition to WARN_ONs to mask/enable helpers for registers to make sure
> callers have the arguments ordered correctly: Pretty much always the
> arguments are static.
> 
> We use WARN_ON(1) a lot in default switch statements though where we
> should always handle all cases. So add a new macro specifically for
> that.
> 
> The idea to use __builtin_constant_p is from Chris Wilson.
> 
> v2: Use the ({}) gcc-ism to avoid the static inline, suggested by
> Dave. My first attempt used __cond as the temp var, which is the same
> used by BUILD_BUG_ON, but with inverted sense. Hilarity ensued, so
> sprinkle i915 into the name.
> 
> Also use a temporary variable to only evaluate the condition once,
> suggested by Damien.
> 
> v3: It's crazy but apparently 32bit gcc can't compile out the
> BUILD_BUG_ON in a lot of cases and just falls over. I have no idea
> why, but until clue grows just disable this nifty idea on 32bit
> builds. Reported by 0-day builder.
> 
> Cc: Damien Lespiau <damien.lespiau at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Dave Gordon <david.s.gordon at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      | 13 ++++++++++++-
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  6 +++---
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  drivers/gpu/drm/i915/intel_uncore.c  |  4 ++--
>  5 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d0e445eca9ce..f44a844a48db 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2347,7 +2347,7 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>  	case POWER_DOMAIN_INIT:
>  		return "INIT";
>  	default:
> -		WARN_ON(1);
> +		MISSING_CASE();

Whilst you are doing this janitorial task, if we add the case value to
the macro, that may speed up debugging when they are hit.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list