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 <[email protected]> > >>>Reviewed-by: Chris Wilson <[email protected]> > >>>Reported-by: Chris Wilson <[email protected]> > >>>Signed-off-by: Joonas Lahtinen <[email protected]> > >>>--- > >>> 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
