drm: Message logging cleanups

PATCH In review Joe Perches 4 participants 10 messages Apr 18, 2011 – Apr 19, 2011

Subsystems (4)

DRM drivers DRM misc Intel i915 Radeon and amdgpu DRM drivers

lore

Share & tools

lore

Joe Perches

author

2011-04-18 03:35 UTC #0 lore raw

cover drm: Message logging cleanups

Reduce the text space required and verify debug arguments.

Joe Perches (2):
drm: Create and use drm_err
drm: Verify debug message arguments

 drivers/gpu/drm/drm_irq.c               |    9 +++++----
 drivers/gpu/drm/drm_stub.c              |   21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_bios.c       |    6 +++---
 drivers/gpu/drm/i915/intel_display.c    |    8 ++++----
 drivers/gpu/drm/radeon/radeon_display.c |    5 +++--
 include/drm/drmP.h                      |   24 +++++++++---------------
 6 files changed, 45 insertions(+), 28 deletions(-)

Signature · 1 line

1.7.4.2.g597a6.dirty

Joe Perches

author

2011-04-18 03:35 UTC #1 lore raw

PATCH 1/2 drm: Create and use drm_err

Reduce drm text size ~1% by using drm_err and
printf extension %pV to emit error messages.

Remove unused macro DRM_MEM_ERROR.

$ size drivers/gpu/drm/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 361159	   9663	    256	 371078	  5a986	drivers/gpu/drm/built-in.o.new
 365416	   9663	    256	 375335	  5ba27	drivers/gpu/drm/built-in.o.old

Signed-off-by Joe Perches <[email protected]>

---
 drivers/gpu/drm/drm_stub.c |   21 +++++++++++++++++++++
 include/drm/drmP.h         |   21 +++++++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)

drivers/gpu/drm/drm_stub.c +21 -0

index 001273d..6d7b083 100644
@@ -62,6 +62,26 @@ struct idr drm_minors_idr;
 struct class *drm_class;
 struct proc_dir_entry *drm_proc_root;
 struct dentry *drm_debugfs_root;
+
+int drm_err(const char *func, const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, format);
+
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	r = printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* %pV", func, &vaf);
+
+	va_end(args);
+
+	return r;
+}
+EXPORT_SYMBOL(drm_err);
+
 void drm_ut_debug_printk(unsigned int request_level,
 			 const char *prefix,
 			 const char *function_name,

@@ -78,6 +98,7 @@ void drm_ut_debug_printk(unsigned int request_level,
 	}
 }
 EXPORT_SYMBOL(drm_ut_debug_printk);
+
 static int drm_minor_get_id(struct drm_device *dev, int type)
 {
 	int new_id;

include/drm/drmP.h +7 ~~-14~~

index 202424d..22db51d 100644
@@ -126,6 +126,9 @@ extern void drm_ut_debug_printk(unsigned int request_level,
 				const char *prefix,
 				const char *function_name,
 				const char *format, ...);
+extern __attribute__((format (printf, 2, 3)))
+int drm_err(const char *func, const char *format, ...);
+
 /***********************************************************************/
 /** \name DRM template customization defaults */
 /*@{*/

@@ -181,21 +184,11 @@ extern void drm_ut_debug_printk(unsigned int request_level,
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#define DRM_ERROR(fmt, arg...) \
-	printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* " fmt , __func__ , ##arg)
-
-/**
- * Memory error output.
- *
- * \param area memory area where the error occurred.
- * \param fmt printf() like format string.
- * \param arg arguments
- */
-#define DRM_MEM_ERROR(area, fmt, arg...) \
-	printk(KERN_ERR "[" DRM_NAME ":%s:%s] *ERROR* " fmt , __func__, \
-	       drm_mem_stats[area].name , ##arg)
+#define DRM_ERROR(fmt, ...)				\
+	drm_err(__func__, fmt, ##__VA_ARGS__)
 
-#define DRM_INFO(fmt, arg...)  printk(KERN_INFO "[" DRM_NAME "] " fmt , ##arg)
+#define DRM_INFO(fmt, ...)				\
+	printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
 
 /**
  * Debug output.

Signature · 1 line

1.7.4.2.g597a6.dirty

Ian Romanick

2011-04-18 22:56 UTC #2 lore raw

Re: [PATCH 1/2] drm: Create and use drm_err

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 27 lines · Joe Perches

Reduce drm text size ~1% by using drm_err and
printf extension %pV to emit error messages.

Remove unused macro DRM_MEM_ERROR.

$ size drivers/gpu/drm/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 361159	   9663	    256	 371078	  5a986	drivers/gpu/drm/built-in.o.new
 365416	   9663	    256	 375335	  5ba27	drivers/gpu/drm/built-in.o.old

Signed-off-by Joe Perches <[email protected]>

---
 drivers/gpu/drm/drm_stub.c |   21 +++++++++++++++++++++
 include/drm/drmP.h         |   21 +++++++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)

drivers/gpu/drm/drm_stub.c +3 -0

index 001273d..6d7b083 100644
@@ -62,6 +62,26 @@ struct idr drm_minors_idr;
 struct class *drm_class;
 struct proc_dir_entry *drm_proc_root;
 struct dentry *drm_debugfs_root;
+
+int drm_err(const char *func, const char *format, ...)
+{

drivers/gpu/drm/drm_stub.c struct idr drm_minors_idr; · line 77

Quoted 10 lines

drivers/gpu/drm/drm_stub.c +10 -0

+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, format);
+
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	r = printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* %pV", func, &vaf);

This code has ben reorganized a lot over the years, so this comment may be bogus. However...

I believe that DRM_NAME is a define, and drm_stub.c is in common code. As a result, won't this change cause something different to get logged?

Quoted 48 lines

drivers/gpu/drm/drm_stub.c +8 -0

+
+	va_end(args);
+
+	return r;
+}
+EXPORT_SYMBOL(drm_err);
+
 void drm_ut_debug_printk(unsigned int request_level,
 			 const char *prefix,
 			 const char *function_name,

@@ -78,6 +98,7 @@ void drm_ut_debug_printk(unsigned int request_level,
 	}
 }
 EXPORT_SYMBOL(drm_ut_debug_printk);
+
 static int drm_minor_get_id(struct drm_device *dev, int type)
 {
 	int new_id;

include/drm/drmP.h +3 ~~-12~~

index 202424d..22db51d 100644
@@ -126,6 +126,9 @@ extern void drm_ut_debug_printk(unsigned int request_level,
 				const char *prefix,
 				const char *function_name,
 				const char *format, ...);
+extern __attribute__((format (printf, 2, 3)))
+int drm_err(const char *func, const char *format, ...);
+
 /***********************************************************************/
 /** \name DRM template customization defaults */
 /*@{*/

@@ -181,21 +184,11 @@ extern void drm_ut_debug_printk(unsigned int request_level,
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#define DRM_ERROR(fmt, arg...) \
-	printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* " fmt , __func__ , ##arg)
-
-/**
- * Memory error output.
- *
- * \param area memory area where the error occurred.
- * \param fmt printf() like format string.
- * \param arg arguments
- */
-#define DRM_MEM_ERROR(area, fmt, arg...) \
-	printk(KERN_ERR "[" DRM_NAME ":%s:%s] *ERROR* " fmt , __func__, \

include/drm/drmP.h extern void drm_ut_debug_printk(unsigned int request_level, · line 194

Quoted 10 lines

include/drm/drmP.h +4 -2

-	       drm_mem_stats[area].name , ##arg)
+#define DRM_ERROR(fmt, ...)				\
+	drm_err(__func__, fmt, ##__VA_ARGS__)
 
-#define DRM_INFO(fmt, arg...)  printk(KERN_INFO "[" DRM_NAME "] " fmt , ##arg)
+#define DRM_INFO(fmt, ...)				\
+	printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
 
 /**
  * Debug output.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2swbEACgkQX1gOwKyEAw+w/ACfd2QMJWOQU5f9sgavPXkfPPfW GygAn2D8bjKkIV8wOXYC1fOI9w4DPWHj
=q3FM
-----END PGP SIGNATURE-----

Dave Airlie

2011-04-18 23:00 UTC #3 lore raw

Re: [PATCH 1/2] drm: Create and use drm_err

Quoted 45 lines · Ian Romanick

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 27 lines · Joe Perches
Reduce drm text size ~1% by using drm_err and
printf extension %pV to emit error messages.
Remove unused macro DRM_MEM_ERROR.
$ size drivers/gpu/drm/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 361159	   9663	    256	 371078	  5a986	drivers/gpu/drm/built-in.o.new
 365416	   9663	    256	 375335	  5ba27	drivers/gpu/drm/built-in.o.old
Signed-off-by Joe Perches <[email protected]>
---
 drivers/gpu/drm/drm_stub.c |   21 +++++++++++++++++++++
 include/drm/drmP.h         |   21 +++++++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_stub.c +3 -0
index 001273d..6d7b083 100644
@@ -62,6 +62,26 @@ struct idr drm_minors_idr;
 struct class *drm_class;
 struct proc_dir_entry *drm_proc_root;
 struct dentry *drm_debugfs_root;
+
+int drm_err(const char *func, const char *format, ...)
+{
drivers/gpu/drm/drm_stub.c struct idr drm_minors_idr; · line 77
Quoted 10 lines
drivers/gpu/drm/drm_stub.c +10 -0
+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, format);
+
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	r = printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* %pV", func, &vaf);
This code has ben reorganized a lot over the years, so this comment may be bogus. However...

Quoted 2 lines

I believe that DRM_NAME is a define, and drm_stub.c is in common code. As a result, won't this change cause something different to get logged?

No I think we hacked that out a long time past thankfully, so its always "drm" now.

Dave.

Joe Perches

author

2011-04-18 23:05 UTC #4 lore raw

Re: [PATCH 1/2] drm: Create and use drm_err

In reply to Ian Romanick

Quoted 2 lines · Ian Romanick

I believe that DRM_NAME is a define, and drm_stub.c is in common code. As a result, won't this change cause something different to get logged?

Nope. DRM_NAME is #defined only once.

include/drm/drm.h:#define DRM_NAME "drm"

The only real reason to do this is the "*ERROR*"
used in:

Quoted 2 lines

include/drm/drmP.h extern void drm_ut_debug_printk(unsigned int request_level, · line 185
Quoted 2 lines
include/drm/drmP.h +0 -2
-#define DRM_ERROR(fmt, arg...) \
-	printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* " fmt , __func__ , ##arg)

It'd be more or less similar to use:

#define DRM_ERROR(fmt, ...)	\
	pr_err("%s: " fmt, __func__, ##__VA_ARGS__)

or just convert DRM_ERROR to pr_err and DRM_INFO to pr_info.

Joe Perches

author

2011-04-18 03:35 UTC #5 lore raw

PATCH 2/2 drm: Verify debug message arguments

In reply to Joe Perches

Add __attribute__((format (printf, 4, 5))) to drm_ut_debug_printk and fix fallout.

Signed-off-by Joe Perches <[email protected]>

---
 drivers/gpu/drm/drm_irq.c               |    9 +++++----
 drivers/gpu/drm/i915/intel_bios.c       |    6 +++---
 drivers/gpu/drm/i915/intel_display.c    |    8 ++++----
 drivers/gpu/drm/radeon/radeon_display.c |    5 +++--
 include/drm/drmP.h                      |    3 ++-
 5 files changed, 17 insertions(+), 14 deletions(-)

drivers/gpu/drm/drm_irq.c +5 -4

index 741457b..62ced75 100644
@@ -684,10 +684,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
 	 */
 	*vblank_time = ns_to_timeval(timeval_to_ns(&raw_time) - delta_ns);
 
-	DRM_DEBUG("crtc %d : v %d p(%d,%d)@ %d.%d -> %d.%d [e %d us, %d rep]\n",
-		  crtc, (int) vbl_status, hpos, vpos, raw_time.tv_sec,
-		  raw_time.tv_usec, vblank_time->tv_sec, vblank_time->tv_usec,
-		  (int) duration_ns/1000, i);
+	DRM_DEBUG("crtc %d : v %d p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n",
+		  crtc, (int)vbl_status, hpos, vpos,
+		  (long)raw_time.tv_sec, (long)raw_time.tv_usec,
+		  (long)vblank_time->tv_sec, (long)vblank_time->tv_usec,
+		  (int)duration_ns/1000, i);
 
 	vbl_status = DRM_VBLANKTIME_SCANOUTPOS_METHOD;
 	if (invbl)

drivers/gpu/drm/i915/intel_bios.c +3 -3

index fb5b4d4..927442a 100644
@@ -214,9 +214,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	    i915_lvds_downclock) {
 		dev_priv->lvds_downclock_avail = 1;
 		dev_priv->lvds_downclock = temp_downclock;
-		DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
-				"Normal Clock %dKHz, downclock %dKHz\n",
-				temp_downclock, panel_fixed_mode->clock);
+		DRM_DEBUG_KMS("LVDS downclock is found in VBT. "
+			      "Normal Clock %dKHz, downclock %dKHz\n",
+			      temp_downclock, panel_fixed_mode->clock);
 	}
 	return;
 }

drivers/gpu/drm/i915/intel_display.c +4 -4

index 432fc04..63bc2af 100644
@@ -3497,11 +3497,11 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
 		1000;
 	entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
 
-	DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
+	DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
 
 	wm_size = fifo_size - (entries_required + wm->guard_size);
 
-	DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
+	DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
 
 	/* Don't promote wm_size to unsigned... */
 	if (wm_size > (long)wm->max_wm)

@@ -3820,13 +3820,13 @@ static bool g4x_check_srwm(struct drm_device *dev,
 		      display_wm, cursor_wm);
 
 	if (display_wm > display->max_wm) {
-		DRM_DEBUG_KMS("display watermark is too large(%d), disabling\n",
+		DRM_DEBUG_KMS("display watermark is too large(%d/%ld), disabling\n",
 			      display_wm, display->max_wm);
 		return false;
 	}
 
 	if (cursor_wm > cursor->max_wm) {
-		DRM_DEBUG_KMS("cursor watermark is too large(%d), disabling\n",
+		DRM_DEBUG_KMS("cursor watermark is too large(%d/%ld), disabling\n",
 			      cursor_wm, cursor->max_wm);
 		return false;
 	}

drivers/gpu/drm/radeon/radeon_display.c +3 -2

index bdbab5c..0671934 100644
@@ -1087,8 +1087,9 @@ void radeon_compute_pll_legacy(struct radeon_pll *pll,
 	*frac_fb_div_p = best_frac_feedback_div;
 	*ref_div_p = best_ref_div;
 	*post_div_p = best_post_div;
-	DRM_DEBUG_KMS("%d %d, pll dividers - fb: %d.%d ref: %d, post %d\n",
-		      freq, best_freq / 1000, best_feedback_div, best_frac_feedback_div,
+	DRM_DEBUG_KMS("%lld %d, pll dividers - fb: %d.%d ref: %d, post %d\n",
+		      (long long)freq,
+		      best_freq / 1000, best_feedback_div, best_frac_feedback_div,
 		      best_ref_div, best_post_div);
 
 }

include/drm/drmP.h +2 -1

index 22db51d..4ab866e 100644
@@ -122,7 +122,8 @@ struct drm_device;
  * using the DRM_DEBUG_KMS and DRM_DEBUG.
  */
 
-extern void drm_ut_debug_printk(unsigned int request_level,
+extern __attribute__((format (printf, 4, 5)))
+void drm_ut_debug_printk(unsigned int request_level,
 				const char *prefix,
 				const char *function_name,
 				const char *format, ...);

Signature · 1 line

1.7.4.2.g597a6.dirty

Ian Romanick

Reviewed-by on v1 2/2

2011-04-18 23:01 UTC #6 lore raw

Re: [PATCH 2/2] drm: Verify debug message arguments

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 4 lines · Joe Perches

Add __attribute__((format (printf, 4, 5))) to drm_ut_debug_printk and fix fallout.
Signed-off-by Joe Perches <[email protected]>

Aside from the comment below about intel_bios.c,

Reviewed-by Ian Romanick <[email protected]>

especially the changes in intel_display.c. yikes...

Quoted 35 lines

---
 drivers/gpu/drm/drm_irq.c               |    9 +++++----
 drivers/gpu/drm/i915/intel_bios.c       |    6 +++---
 drivers/gpu/drm/i915/intel_display.c    |    8 ++++----
 drivers/gpu/drm/radeon/radeon_display.c |    5 +++--
 include/drm/drmP.h                      |    3 ++-
 5 files changed, 17 insertions(+), 14 deletions(-)

drivers/gpu/drm/drm_irq.c +5 -4

index 741457b..62ced75 100644
@@ -684,10 +684,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc,
 	 */
 	*vblank_time = ns_to_timeval(timeval_to_ns(&raw_time) - delta_ns);
 
-	DRM_DEBUG("crtc %d : v %d p(%d,%d)@ %d.%d -> %d.%d [e %d us, %d rep]\n",
-		  crtc, (int) vbl_status, hpos, vpos, raw_time.tv_sec,
-		  raw_time.tv_usec, vblank_time->tv_sec, vblank_time->tv_usec,
-		  (int) duration_ns/1000, i);
+	DRM_DEBUG("crtc %d : v %d p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n",
+		  crtc, (int)vbl_status, hpos, vpos,
+		  (long)raw_time.tv_sec, (long)raw_time.tv_usec,
+		  (long)vblank_time->tv_sec, (long)vblank_time->tv_usec,
+		  (int)duration_ns/1000, i);
 
 	vbl_status = DRM_VBLANKTIME_SCANOUTPOS_METHOD;
 	if (invbl)

drivers/gpu/drm/i915/intel_bios.c +0 -0

index fb5b4d4..927442a 100644
@@ -214,9 +214,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	    i915_lvds_downclock) {
 		dev_priv->lvds_downclock_avail = 1;

drivers/gpu/drm/i915/intel_bios.c parse_lfp_panel_data(struct drm_i915_private *dev_priv, · line 222

Quoted 10 lines

drivers/gpu/drm/i915/intel_bios.c +3 -3

 		dev_priv->lvds_downclock = temp_downclock;
-		DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
-				"Normal Clock %dKHz, downclock %dKHz\n",
-				temp_downclock, panel_fixed_mode->clock);
+		DRM_DEBUG_KMS("LVDS downclock is found in VBT. "
+			      "Normal Clock %dKHz, downclock %dKHz\n",
+			      temp_downclock, panel_fixed_mode->clock);
 	}
 	return;
 }

Does this hunk only change white space, or am I missing something?

Quoted 55 lines

drivers/gpu/drm/i915/intel_display.c +4 -4

index 432fc04..63bc2af 100644
@@ -3497,11 +3497,11 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
 		1000;
 	entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
 
-	DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
+	DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
 
 	wm_size = fifo_size - (entries_required + wm->guard_size);
 
-	DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
+	DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
 
 	/* Don't promote wm_size to unsigned... */
 	if (wm_size > (long)wm->max_wm)

@@ -3820,13 +3820,13 @@ static bool g4x_check_srwm(struct drm_device *dev,
 		      display_wm, cursor_wm);
 
 	if (display_wm > display->max_wm) {
-		DRM_DEBUG_KMS("display watermark is too large(%d), disabling\n",
+		DRM_DEBUG_KMS("display watermark is too large(%d/%ld), disabling\n",
 			      display_wm, display->max_wm);
 		return false;
 	}
 
 	if (cursor_wm > cursor->max_wm) {
-		DRM_DEBUG_KMS("cursor watermark is too large(%d), disabling\n",
+		DRM_DEBUG_KMS("cursor watermark is too large(%d/%ld), disabling\n",
 			      cursor_wm, cursor->max_wm);
 		return false;
 	}

drivers/gpu/drm/radeon/radeon_display.c +3 -2

index bdbab5c..0671934 100644
@@ -1087,8 +1087,9 @@ void radeon_compute_pll_legacy(struct radeon_pll *pll,
 	*frac_fb_div_p = best_frac_feedback_div;
 	*ref_div_p = best_ref_div;
 	*post_div_p = best_post_div;
-	DRM_DEBUG_KMS("%d %d, pll dividers - fb: %d.%d ref: %d, post %d\n",
-		      freq, best_freq / 1000, best_feedback_div, best_frac_feedback_div,
+	DRM_DEBUG_KMS("%lld %d, pll dividers - fb: %d.%d ref: %d, post %d\n",
+		      (long long)freq,
+		      best_freq / 1000, best_feedback_div, best_frac_feedback_div,
 		      best_ref_div, best_post_div);
 
 }

include/drm/drmP.h +0 -0

index 22db51d..4ab866e 100644

include/drm/drmP.h struct drm_device; · line 129

Quoted 9 lines

include/drm/drmP.h +2 -1

@@ -122,7 +122,8 @@ struct drm_device;
  * using the DRM_DEBUG_KMS and DRM_DEBUG.
  */
 
-extern void drm_ut_debug_printk(unsigned int request_level,
+extern __attribute__((format (printf, 4, 5)))
+void drm_ut_debug_printk(unsigned int request_level,
 				const char *prefix,
 				const char *function_name,
 				const char *format, ...);

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2swrgACgkQX1gOwKyEAw+b8wCfdNqYUGinwLaXk7PeNNu8ExXe 39EAoIWQPkZ3z8Au22tTM3cXOQlIuAH4
=4RWh
-----END PGP SIGNATURE-----

Joe Perches

author

2011-04-18 23:09 UTC #7 lore raw

Re: [PATCH 2/2] drm: Verify debug message arguments

Quoted 6 lines · Ian Romanick

Quoted 6 lines

drivers/gpu/drm/i915/intel_bios.c +0 -2

@@ -214,9 +214,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	    i915_lvds_downclock) {
 		dev_priv->lvds_downclock_avail = 1;
 		dev_priv->lvds_downclock = temp_downclock;
-		DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
-				"Normal Clock %dKHz, downclock %dKHz\n",

Quoted 8 lines

drivers/gpu/drm/i915/intel_bios.c parse_lfp_panel_data(struct drm_i915_private *dev_priv, · line 222
Quoted 7 lines
drivers/gpu/drm/i915/intel_bios.c +3 -1
-				temp_downclock, panel_fixed_mode->clock);
+		DRM_DEBUG_KMS("LVDS downclock is found in VBT. "
+			      "Normal Clock %dKHz, downclock %dKHz\n",
+			      temp_downclock, panel_fixed_mode->clock);
 	}
 	return;
 }
Does this hunk only change white space, or am I missing something?

No, you're right. It's just whitespace.
I prefer arguments aligned to open paren.

Marcin Slusarz

2011-04-19 16:26 UTC #8 lore raw

Re: [PATCH 2/2] drm: Verify debug message arguments

Quoted 16 lines · Joe Perches

Quoted 6 lines · Ian Romanick
Quoted 6 lines
drivers/gpu/drm/i915/intel_bios.c +0 -2
@@ -214,9 +214,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	    i915_lvds_downclock) {
 		dev_priv->lvds_downclock_avail = 1;
 		dev_priv->lvds_downclock = temp_downclock;
-		DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
-				"Normal Clock %dKHz, downclock %dKHz\n",
Quoted 8 lines
drivers/gpu/drm/i915/intel_bios.c parse_lfp_panel_data(struct drm_i915_private *dev_priv, · line 222
Quoted 7 lines
drivers/gpu/drm/i915/intel_bios.c +3 -1
-				temp_downclock, panel_fixed_mode->clock);
+		DRM_DEBUG_KMS("LVDS downclock is found in VBT. "
+			      "Normal Clock %dKHz, downclock %dKHz\n",
+			      temp_downclock, panel_fixed_mode->clock);
 	}
 	return;
 }
Does this hunk only change white space, or am I missing something?

Quoted 2 lines

No, you're right. It's just whitespace.
I prefer arguments aligned to open paren.

It's not just whitespace. Look at the end of first line.

Marcin

Joe Perches

author

2011-04-19 16:31 UTC #9 lore raw

Re: [PATCH 2/2] drm: Verify debug message arguments

Quoted 9 lines · Marcin Slusarz

Quoted 8 lines · Joe Perches
Quoted 7 lines · Ian Romanick
drivers/gpu/drm/i915/intel_bios.c parse_lfp_panel_data(struct drm_i915_private *dev_priv, · line 219
Quoted 6 lines
drivers/gpu/drm/i915/intel_bios.c +3 -3
-		DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
-				"Normal Clock %dKHz, downclock %dKHz\n",
-				temp_downclock, panel_fixed_mode->clock);
+		DRM_DEBUG_KMS("LVDS downclock is found in VBT. "
+			      "Normal Clock %dKHz, downclock %dKHz\n",
+			      temp_downclock, panel_fixed_mode->clock);
Does this hunk only change white space, or am I missing something?

Quoted 3 lines

Quoted 2 lines
No, you're right. It's just whitespace.
I prefer arguments aligned to open paren.
It's not just whitespace. Look at the end of first line.

Heh. Thanks Marcin.

Teach me to just at the code in the patch.

It's a format error as the first quoted string has a comma at the end so the arguments after the comma were ignored.