Re: [PATCH v3 1/2] drm/i915: Simplify _STATE_ debug macros

[Dave Gordon <david.s.gordon@intel.com>]

On 18/12/15 12:27, Joonas Lahtinen wrote:
> Take advantage of WARN return value to simplify the flow.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1d28d90..5a5a3e0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -87,23 +87,18 @@
>    */
>   #define I915_STATE_WARN(condition, format...) ({			\
>   	int __ret_warn_on = !!(condition);				\
> -	if (unlikely(__ret_warn_on)) {					\
> -		if (i915.verbose_state_checks)				\
> -			WARN(1, format);				\
> -		else 							\
> +	if (unlikely(__ret_warn_on))					\
> +		if (!WARN(i915.verbose_state_checks, format))		\
>   			DRM_ERROR(format);				\
> -	}								\
>   	unlikely(__ret_warn_on);					\
>   })
>
>   #define I915_STATE_WARN_ON(condition) ({				\
>   	int __ret_warn_on = !!(condition);				\
> -	if (unlikely(__ret_warn_on)) {					\
> -		if (i915.verbose_state_checks)				\
> -			WARN(1, "WARN_ON(" #condition ")\n");		\
> -		else 							\
> +	if (unlikely(__ret_warn_on))					\
> +		if (!WARN(i915.verbose_state_checks,			\
> +			  "WARN_ON(" #condition ")\n"))			\
>   			DRM_ERROR("WARN_ON(" #condition ")\n");		\

These last two lines still have the text of the condition as part of a 
format string :(

For compile-testing, you might want to change:

   static void lpt_bend_clkout_dp(struct drm_i915_private *dev_priv
   ...
     if (WARN_ON(steps % 5 != 0))
       return;

to use I915_STATE_WARN_ON() instead of WARN_ON, then you should get a 
compile-time warning if the '%' ends up in the format string.

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx