[PATCH v3 0/2] Optimize WARN_ON macros

[PATCH v3 0/2] Optimize WARN_ON macros

[Joonas Lahtinen]

v3:
- Fixed potential OOPS scenario with modulo in condition
  (Dave Gordon)

Joonas Lahtinen (2):
  drm/i915: Simplify _STATE_ debug macros
  drm/i915: Compile-time concatenate WARN_ON macro strings

 drivers/gpu/drm/i915/i915_drv.h | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

-- 
2.4.3

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

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

[Joonas Lahtinen]

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");		\
-	}								\
 	unlikely(__ret_warn_on);					\
 })
 
-- 
2.4.3

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

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

[Dave Gordon]

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

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

[Joonas Lahtinen]

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

> .Dave.
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

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

[Dave Gordon]

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

Re: [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@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!

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Joonas Lahtinen]

On ma, 2015-12-21 at 14:41 +0100, Daniel Vetter wrote:
> 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@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!
> 
> Yeah I think it makes sense to fix that. I'll wait for v4.

Ok, to be perfectly clear; This is patch 1/2, which does as the commit
message specifies, it simplifies the function flow, patch 2/2 of the
series changes the string construction and addresses the mentioned
problem. Sorry for being little bit unclear about that on the first
comment.

Regards, Joonas

> -Daniel
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

[PATCH v3 2/2] drm/i915: Compile-time concatenate WARN_ON macro strings

[Joonas Lahtinen]

Using __stringify(x) instead of #x adds support for macros as
a parameter and compile-time concatenation reduces the runtime
overhead.

Slightly increases the .text size but should not matter.

v2:
- Define I915_STATE_WARN_ON though I915_STATE_WARN
  (Bikeshed inspiration by Chris)

v3:
- More specific commit message

v4:
- Do not directly pass arbitary string as format, instead
  guard with "%s" (Dave)

Cc: Rob Clark <robdclark@gmail.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5a5a3e0..664ceb4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -69,11 +69,11 @@
 		BUILD_BUG_ON(__i915_warn_cond); \
 	WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
 #else
-#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
+#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
 #endif
 
 #undef WARN_ON_ONCE
-#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
+#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
 
 #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
 			     (long) (x), __func__);
@@ -93,14 +93,8 @@
 	unlikely(__ret_warn_on);					\
 })
 
-#define I915_STATE_WARN_ON(condition) ({				\
-	int __ret_warn_on = !!(condition);				\
-	if (unlikely(__ret_warn_on))					\
-		if (!WARN(i915.verbose_state_checks,			\
-			  "WARN_ON(" #condition ")\n"))			\
-			DRM_ERROR("WARN_ON(" #condition ")\n");		\
-	unlikely(__ret_warn_on);					\
-})
+#define I915_STATE_WARN_ON(x)						\
+	I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
 
 static inline const char *yesno(bool v)
 {
-- 
2.4.3

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

Re: [PATCH v3 2/2] drm/i915: Compile-time concatenate WARN_ON macro strings

[Daniel Vetter]

On Fri, Dec 18, 2015 at 02:27:27PM +0200, Joonas Lahtinen wrote:
> Using __stringify(x) instead of #x adds support for macros as
> a parameter and compile-time concatenation reduces the runtime
> overhead.
> 
> Slightly increases the .text size but should not matter.
> 
> v2:
> - Define I915_STATE_WARN_ON though I915_STATE_WARN
>   (Bikeshed inspiration by Chris)
> 
> v3:
> - More specific commit message
> 
> v4:
> - Do not directly pass arbitary string as format, instead
>   guard with "%s" (Dave)
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Both patches merged to dinq, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5a5a3e0..664ceb4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -69,11 +69,11 @@
>  		BUILD_BUG_ON(__i915_warn_cond); \
>  	WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
>  #else
> -#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
> +#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
>  #endif
>  
>  #undef WARN_ON_ONCE
> -#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
> +#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")")
>  
>  #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
>  			     (long) (x), __func__);
> @@ -93,14 +93,8 @@
>  	unlikely(__ret_warn_on);					\
>  })
>  
> -#define I915_STATE_WARN_ON(condition) ({				\
> -	int __ret_warn_on = !!(condition);				\
> -	if (unlikely(__ret_warn_on))					\
> -		if (!WARN(i915.verbose_state_checks,			\
> -			  "WARN_ON(" #condition ")\n"))			\
> -			DRM_ERROR("WARN_ON(" #condition ")\n");		\
> -	unlikely(__ret_warn_on);					\
> -})
> +#define I915_STATE_WARN_ON(x)						\
> +	I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
>  
>  static inline const char *yesno(bool v)
>  {
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 3c71db89fc37b6061ce6b070ce73f89155da5f20 drm-intel-nightly: 2015y-12m-18d-09h-42m-49s UTC integration manifest

Test igt@kms_flip@basic-flip-vs-wf_vblank on ivb-t430s pass -> dmesg-warn
Test igt@kms_flip@basic-flip-vs-wf_vblank on bsw-nuc-2 pass -> dmesg-warn
Test igt@gem_storedw_loop@basic-render on skl-i7k-2 pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@hang-read-crc-pipe-a on ivb-t430s dmesg-warn -> pass
Test igt@gem_mmap@basic on byt-nuc dmesg-warn -> pass
Test igt@gem_mmap_gtt@basic-write on bsw-nuc-2 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a on snb-x220t pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on ilk-hp8440p pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on snb-x220t dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-a on byt-nuc dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-c on bdw-nuci7 pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c on skl-i5k-2 dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-modeset on hsw-brixbox dmesg-warn -> pass

bdw-nuci7        total:135  pass:124  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
hsw-xps12        total:132  pass:125  dwarn:3   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:99   dwarn:1   dfail:0   fail:2   skip:33 
ivb-t430s        total:135  pass:128  dwarn:1   dfail:1   fail:1   skip:4  
skl-i5k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:122  dwarn:5   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_717/

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 3c71db89fc37b6061ce6b070ce73f89155da5f20 drm-intel-nightly: 2015y-12m-18d-09h-42m-49s UTC integration manifest

Test gem_mmap:
        Subgroup basic:
                dmesg-warn -> PASS       (byt-nuc)
Test gem_mmap_gtt:
        Subgroup basic-write:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (hsw-brixbox)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (bsw-nuc-2)
                pass       -> DMESG-WARN (ivb-t430s)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (ivb-t430s)
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)
                pass       -> DMESG-WARN (ilk-hp8440p)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (skl-i5k-2)
                pass       -> DMESG-WARN (bdw-nuci7)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (snb-x220t)

bdw-nuci7        total:135  pass:124  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
hsw-xps12        total:132  pass:125  dwarn:3   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:99   dwarn:1   dfail:0   fail:2   skip:33 
ivb-t430s        total:135  pass:128  dwarn:1   dfail:1   fail:1   skip:4  
skl-i5k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:122  dwarn:5   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_717/

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