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

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

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@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.
>>
>
> 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!

.Dave.

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