[Intel-gfx] [PATCH v3 1/2] drm/i915: Simplify _STATE_ debug macros

[Daniel Vetter]

On Mon, Dec 21, 2015 at 11:53:52AM +0000, Dave Gordon wrote:
> On 21/12/15 08:11, Joonas Lahtinen wrote:
> >On pe, 2015-12-18 at 16:18 +0000, Dave Gordon wrote:
> >>On 18/12/15 12:27, Joonas Lahtinen wrote:
> >>>Take advantage of WARN return value to simplify the flow.
> >>>
> >>>Cc: Rob Clark <robdclark at gmail.com>
> >>>Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>Reported-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>Signed-off-by: Joonas Lahtinen <joonas.lahtinen at 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.
> >>
> >
> >This is just a patch to convert the old macros to different order
> >before changing them. The way of constructing the strings is intact.
> >
> >Regards, Joonas
> 
> Yes, I agree, you didn't break them -- they were already wrong!

Yeah I think it makes sense to fix that. I'll wait for v4.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch