Skip to contentSkip to conversation

drm: misc cleanup

PATCH Applied Rob Clark 3 participants 7 messages Oct 13, 2015 – Oct 16, 2015

Subsystems (2)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

PATCH 1/1 drm: misc cleanup

Drop unused drm_atomic and fix comment for drm_debug.

---
 drivers/gpu/drm/drm_drv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/gpu/drm/drm_drv.c +1 -3
index 53d09a1..3a8d598 100644
@@ -37,11 +37,9 @@
 #include "drm_legacy.h"
 #include "drm_internal.h"
 
-unsigned int drm_debug = 0;	/* 1 to enable debug output */
+unsigned int drm_debug = 0;	/* bitmask of DRM_UT_x */
 EXPORT_SYMBOL(drm_debug);
 
-bool drm_atomic = 0;
-
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
Signature · 6 lines
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#1 lore raw

PATCH 1/1 drm/dp: add DPCD logging

Add a new drm_debug bit for turning on DPCD logging, to aid debugging with troublesome monitors.

---
 drivers/gpu/drm/drm_dp_helper.c | 66 ++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h              |  6 ++++
 2 files changed, 58 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +52 -14
index 291734e..8e17f55 100644
@@ -159,6 +159,46 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
+static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+	size_t ret;
+
+	mutex_lock(&aux->hw_mutex);
+
+	DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
+			msg->request, msg->address, msg->size);
+	if (unlikely(drm_debug & DRM_UT_DPCD)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_WRITE:
+		case DP_AUX_I2C_WRITE:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, msg->size, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ret = aux->transfer(aux, msg);
+
+	DRM_DEBUG_DPCD("%s: reply=0x%02x, size=%zu\n", aux->name, msg->reply, ret);
+	if (unlikely(drm_debug & DRM_UT_DPCD)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_READ:
+		case DP_AUX_I2C_READ:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, ret, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	mutex_unlock(&aux->hw_mutex);
+
+	return ret;
+}
+
 #define AUX_RETRY_INTERVAL 500 /* us */
 
 /**
@@ -194,9 +234,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 	 */
 	for (retry = 0; retry < 32; retry++) {
 
-		mutex_lock(&aux->hw_mutex);
-		err = aux->transfer(aux, &msg);
-		mutex_unlock(&aux->hw_mutex);
+		err = aux_transfer(aux, &msg);
 		if (err < 0) {
 			if (err == -EBUSY)
 				continue;
@@ -212,15 +250,17 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 			return err;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
 			return -EIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
+			DRM_DEBUG_DPCD("native defer\n");
 			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
 			break;
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_DEBUG_DPCD("too many retries, giving up\n");
 	return -EIO;
 }
 
@@ -530,14 +570,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
 
 	for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
-		mutex_lock(&aux->hw_mutex);
-		ret = aux->transfer(aux, msg);
-		mutex_unlock(&aux->hw_mutex);
+		ret = aux_transfer(aux, msg);
 		if (ret < 0) {
 			if (ret == -EBUSY)
 				continue;
 
-			DRM_DEBUG_KMS("transaction failed: %d\n", ret);
+			DRM_DEBUG_DPCD("transaction failed: %d\n", ret);
 			return ret;
 		}
 
@@ -551,11 +589,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			break;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
-			DRM_DEBUG_KMS("native nack (result=%d, size=%zu)\n", ret, msg->size);
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", ret, msg->size);
 			return -EREMOTEIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
-			DRM_DEBUG_KMS("native defer\n");
+			DRM_DEBUG_DPCD("native defer\n");
 			/*
 			 * We could check for I2C bit rate capabilities and if
 			 * available adjust this interval. We could also be
@@ -582,12 +620,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return ret;
 
 		case DP_AUX_I2C_REPLY_NACK:
-			DRM_DEBUG_KMS("I2C nack (result=%d, size=%zu\n", ret, msg->size);
+			DRM_DEBUG_DPCD("I2C nack (result=%d, size=%zu\n", ret, msg->size);
 			aux->i2c_nack_count++;
 			return -EREMOTEIO;
 
 		case DP_AUX_I2C_REPLY_DEFER:
-			DRM_DEBUG_KMS("I2C defer\n");
+			DRM_DEBUG_DPCD("I2C defer\n");
 			/* DP Compliance Test 4.2.2.5 Requirement:
 			 * Must have at least 7 retries for I2C defers on the
 			 * transaction to pass this test
@@ -604,7 +642,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_DEBUG_DPCD("too many retries, giving up\n");
 	return -EREMOTEIO;
 }
 
@@ -624,7 +662,7 @@ static int drm_dp_i2c_drain_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *o
 			return err == 0 ? -EPROTO : err;
 
 		if (err < msg.size && err < ret) {
-			DRM_DEBUG_KMS("Partial I2C reply: requested %zu bytes got %d bytes\n",
+			DRM_DEBUG_DPCD("Partial I2C reply: requested %zu bytes got %d bytes\n",
 				      msg.size, err);
 			ret = err;
 		}
include/drm/drmP.h +6 -0
index 8b5ce7c..f6c25cc 100644
@@ -125,6 +125,7 @@ struct dma_buf_attachment;
 #define DRM_UT_KMS		0x04
 #define DRM_UT_PRIME		0x08
 #define DRM_UT_ATOMIC		0x10
+#define DRM_UT_DPCD		0x20
 
 extern __printf(2, 3)
 void drm_ut_debug_printk(const char *function_name,
@@ -217,6 +218,11 @@ void drm_err(const char *format, ...);
 		if (unlikely(drm_debug & DRM_UT_ATOMIC))		\
 			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
+#define DRM_DEBUG_DPCD(fmt, args...)					\
+	do {								\
+		if (unlikely(drm_debug & DRM_UT_DPCD))			\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
+	} while (0)
 
 /*@}*/
 
Signature · 6 lines
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#2 lore raw

Re: [PATCH] drm/dp: add DPCD logging

Quoted 2 lines · Rob Clark

Add a new drm_debug bit for turning on DPCD logging, to aid debugging with troublesome monitors.

I wish you could find some balance between having the error cases logged with normal kms debugging vs. having to have full blown aux traffic debugging to notice everything failed...

At the very least the "too many retries, giving up" should be included in KMS debugging, maybe more.

BR,
Jani.

Quoted 183 lines
---
 drivers/gpu/drm/drm_dp_helper.c | 66 ++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h              |  6 ++++
 2 files changed, 58 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +52 -14
index 291734e..8e17f55 100644
@@ -159,6 +159,46 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
+static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+	size_t ret;
+
+	mutex_lock(&aux->hw_mutex);
+
+	DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
+			msg->request, msg->address, msg->size);
+	if (unlikely(drm_debug & DRM_UT_DPCD)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_WRITE:
+		case DP_AUX_I2C_WRITE:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, msg->size, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ret = aux->transfer(aux, msg);
+
+	DRM_DEBUG_DPCD("%s: reply=0x%02x, size=%zu\n", aux->name, msg->reply, ret);
+	if (unlikely(drm_debug & DRM_UT_DPCD)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_READ:
+		case DP_AUX_I2C_READ:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, ret, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	mutex_unlock(&aux->hw_mutex);
+
+	return ret;
+}
+
 #define AUX_RETRY_INTERVAL 500 /* us */
 
 /**
@@ -194,9 +234,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 	 */
 	for (retry = 0; retry < 32; retry++) {
 
-		mutex_lock(&aux->hw_mutex);
-		err = aux->transfer(aux, &msg);
-		mutex_unlock(&aux->hw_mutex);
+		err = aux_transfer(aux, &msg);
 		if (err < 0) {
 			if (err == -EBUSY)
 				continue;
@@ -212,15 +250,17 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 			return err;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
 			return -EIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
+			DRM_DEBUG_DPCD("native defer\n");
 			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
 			break;
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_DEBUG_DPCD("too many retries, giving up\n");
 	return -EIO;
 }
 
@@ -530,14 +570,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
 
 	for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
-		mutex_lock(&aux->hw_mutex);
-		ret = aux->transfer(aux, msg);
-		mutex_unlock(&aux->hw_mutex);
+		ret = aux_transfer(aux, msg);
 		if (ret < 0) {
 			if (ret == -EBUSY)
 				continue;
 
-			DRM_DEBUG_KMS("transaction failed: %d\n", ret);
+			DRM_DEBUG_DPCD("transaction failed: %d\n", ret);
 			return ret;
 		}
 
@@ -551,11 +589,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			break;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
-			DRM_DEBUG_KMS("native nack (result=%d, size=%zu)\n", ret, msg->size);
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", ret, msg->size);
 			return -EREMOTEIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
-			DRM_DEBUG_KMS("native defer\n");
+			DRM_DEBUG_DPCD("native defer\n");
 			/*
 			 * We could check for I2C bit rate capabilities and if
 			 * available adjust this interval. We could also be
@@ -582,12 +620,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return ret;
 
 		case DP_AUX_I2C_REPLY_NACK:
-			DRM_DEBUG_KMS("I2C nack (result=%d, size=%zu\n", ret, msg->size);
+			DRM_DEBUG_DPCD("I2C nack (result=%d, size=%zu\n", ret, msg->size);
 			aux->i2c_nack_count++;
 			return -EREMOTEIO;
 
 		case DP_AUX_I2C_REPLY_DEFER:
-			DRM_DEBUG_KMS("I2C defer\n");
+			DRM_DEBUG_DPCD("I2C defer\n");
 			/* DP Compliance Test 4.2.2.5 Requirement:
 			 * Must have at least 7 retries for I2C defers on the
 			 * transaction to pass this test
@@ -604,7 +642,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_DEBUG_DPCD("too many retries, giving up\n");
 	return -EREMOTEIO;
 }
 
@@ -624,7 +662,7 @@ static int drm_dp_i2c_drain_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *o
 			return err == 0 ? -EPROTO : err;
 
 		if (err < msg.size && err < ret) {
-			DRM_DEBUG_KMS("Partial I2C reply: requested %zu bytes got %d bytes\n",
+			DRM_DEBUG_DPCD("Partial I2C reply: requested %zu bytes got %d bytes\n",
 				      msg.size, err);
 			ret = err;
 		}
include/drm/drmP.h +6 -0
index 8b5ce7c..f6c25cc 100644
@@ -125,6 +125,7 @@ struct dma_buf_attachment;
 #define DRM_UT_KMS		0x04
 #define DRM_UT_PRIME		0x08
 #define DRM_UT_ATOMIC		0x10
+#define DRM_UT_DPCD		0x20
 
 extern __printf(2, 3)
 void drm_ut_debug_printk(const char *function_name,
@@ -217,6 +218,11 @@ void drm_err(const char *format, ...);
 		if (unlikely(drm_debug & DRM_UT_ATOMIC))		\
 			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
+#define DRM_DEBUG_DPCD(fmt, args...)					\
+	do {								\
+		if (unlikely(drm_debug & DRM_UT_DPCD))			\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
+	} while (0)
 
 /*@}*/
 
Signature · 6 lines
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#3 lore raw

Re: [PATCH] drm/dp: add DPCD logging

Quoted 10 lines · Jani Nikula
Quoted 2 lines · Rob Clark

Add a new drm_debug bit for turning on DPCD logging, to aid debugging with troublesome monitors.

I wish you could find some balance between having the error cases logged with normal kms debugging vs. having to have full blown aux traffic debugging to notice everything failed...

At the very least the "too many retries, giving up" should be included in KMS debugging, maybe more.

maybe that should be DRM_ERROR()? Not sure..

For what I was debugging I actually wanted to be able to turn on just the DPCD logging without having to turn on all the KMS debugging.. but I can understand the inverse..

BR,
-R

Quoted 183 lines

BR,
Jani.

Quoted 179 lines
---
 drivers/gpu/drm/drm_dp_helper.c | 66 ++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h              |  6 ++++
 2 files changed, 58 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +52 -14
index 291734e..8e17f55 100644
@@ -159,6 +159,46 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
+static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+     size_t ret;
+
+     mutex_lock(&aux->hw_mutex);
+
+     DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
+                     msg->request, msg->address, msg->size);
+     if (unlikely(drm_debug & DRM_UT_DPCD)) {
+             switch (msg->request & ~DP_AUX_I2C_MOT) {
+             case DP_AUX_NATIVE_WRITE:
+             case DP_AUX_I2C_WRITE:
+                     print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+                                     16, 1, msg->buffer, msg->size, false);
+                     break;
+             default:
+                     break;
+             }
+     }
+
+     ret = aux->transfer(aux, msg);
+
+     DRM_DEBUG_DPCD("%s: reply=0x%02x, size=%zu\n", aux->name, msg->reply, ret);
+     if (unlikely(drm_debug & DRM_UT_DPCD)) {
+             switch (msg->request & ~DP_AUX_I2C_MOT) {
+             case DP_AUX_NATIVE_READ:
+             case DP_AUX_I2C_READ:
+                     print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+                                     16, 1, msg->buffer, ret, false);
+                     break;
+             default:
+                     break;
+             }
+     }
+
+     mutex_unlock(&aux->hw_mutex);
+
+     return ret;
+}
+
 #define AUX_RETRY_INTERVAL 500 /* us */
 
 /**
@@ -194,9 +234,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
       */
      for (retry = 0; retry < 32; retry++) {
 
-             mutex_lock(&aux->hw_mutex);
-             err = aux->transfer(aux, &msg);
-             mutex_unlock(&aux->hw_mutex);
+             err = aux_transfer(aux, &msg);
              if (err < 0) {
                      if (err == -EBUSY)
                              continue;
@@ -212,15 +250,17 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
                      return err;
 
              case DP_AUX_NATIVE_REPLY_NACK:
+                     DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
                      return -EIO;
 
              case DP_AUX_NATIVE_REPLY_DEFER:
+                     DRM_DEBUG_DPCD("native defer\n");
                      usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
                      break;
              }
      }
 
-     DRM_DEBUG_KMS("too many retries, giving up\n");
+     DRM_DEBUG_DPCD("too many retries, giving up\n");
      return -EIO;
 }
 
@@ -530,14 +570,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
      int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
 
      for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
-             mutex_lock(&aux->hw_mutex);
-             ret = aux->transfer(aux, msg);
-             mutex_unlock(&aux->hw_mutex);
+             ret = aux_transfer(aux, msg);
              if (ret < 0) {
                      if (ret == -EBUSY)
                              continue;
 
-                     DRM_DEBUG_KMS("transaction failed: %d\n", ret);
+                     DRM_DEBUG_DPCD("transaction failed: %d\n", ret);
                      return ret;
              }
 
@@ -551,11 +589,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
                      break;
 
              case DP_AUX_NATIVE_REPLY_NACK:
-                     DRM_DEBUG_KMS("native nack (result=%d, size=%zu)\n", ret, msg->size);
+                     DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", ret, msg->size);
                      return -EREMOTEIO;
 
              case DP_AUX_NATIVE_REPLY_DEFER:
-                     DRM_DEBUG_KMS("native defer\n");
+                     DRM_DEBUG_DPCD("native defer\n");
                      /*
                       * We could check for I2C bit rate capabilities and if
                       * available adjust this interval. We could also be
@@ -582,12 +620,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
                      return ret;
 
              case DP_AUX_I2C_REPLY_NACK:
-                     DRM_DEBUG_KMS("I2C nack (result=%d, size=%zu\n", ret, msg->size);
+                     DRM_DEBUG_DPCD("I2C nack (result=%d, size=%zu\n", ret, msg->size);
                      aux->i2c_nack_count++;
                      return -EREMOTEIO;
 
              case DP_AUX_I2C_REPLY_DEFER:
-                     DRM_DEBUG_KMS("I2C defer\n");
+                     DRM_DEBUG_DPCD("I2C defer\n");
                      /* DP Compliance Test 4.2.2.5 Requirement:
                       * Must have at least 7 retries for I2C defers on the
                       * transaction to pass this test
@@ -604,7 +642,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
              }
      }
 
-     DRM_DEBUG_KMS("too many retries, giving up\n");
+     DRM_DEBUG_DPCD("too many retries, giving up\n");
      return -EREMOTEIO;
 }
 
@@ -624,7 +662,7 @@ static int drm_dp_i2c_drain_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *o
                      return err == 0 ? -EPROTO : err;
 
              if (err < msg.size && err < ret) {
-                     DRM_DEBUG_KMS("Partial I2C reply: requested %zu bytes got %d bytes\n",
+                     DRM_DEBUG_DPCD("Partial I2C reply: requested %zu bytes got %d bytes\n",
                                    msg.size, err);
                      ret = err;
              }
include/drm/drmP.h +6 -1
index 8b5ce7c..f6c25cc 100644
@@ -125,6 +125,7 @@ struct dma_buf_attachment;
 #define DRM_UT_KMS           0x04
 #define DRM_UT_PRIME         0x08
 #define DRM_UT_ATOMIC                0x10
+#define DRM_UT_DPCD          0x20
 
 extern __printf(2, 3)
 void drm_ut_debug_printk(const char *function_name,
@@ -217,6 +218,11 @@ void drm_err(const char *format, ...);
              if (unlikely(drm_debug & DRM_UT_ATOMIC))                \
                      drm_ut_debug_printk(__func__, fmt, ##args);     \
      } while (0)
+#define DRM_DEBUG_DPCD(fmt, args...)                                 \
+     do {                                                            \
+             if (unlikely(drm_debug & DRM_UT_DPCD))                  \
+                     drm_ut_debug_printk(__func__, fmt, ##args);     \
+     } while (0)
 
 /*@}*/
 
--

2.1.0

Quoted 7 lines
Quoted 4 lines

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Jani Nikula, Intel Open Source Technology Center

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

#4 lore raw

Re: [PATCH] drm/dp: add DPCD logging

Quoted 6 lines · Rob Clark
Quoted 4 lines · Jani Nikula
Quoted 2 lines · Rob Clark

Add a new drm_debug bit for turning on DPCD logging, to aid debugging with troublesome monitors.

Quoted 8 lines
Quoted 6 lines

I wish you could find some balance between having the error cases logged with normal kms debugging vs. having to have full blown aux traffic debugging to notice everything failed...

At the very least the "too many retries, giving up" should be included in KMS debugging, maybe more.

maybe that should be DRM_ERROR()? Not sure..

I'd be fine with that, especially if you can beat some sense into all the confusion between doing an error and debug logging.

For added complexity, there's some DRM_ERRORs and DRM_DEBUG_KMSs hidden in the ->transfer functions too. See e.g. intel_dp_aux_ch() that gets called by our ->transfer function intel_dp_aux_transfer(), both in i915/intel_dp.c...

BR,
Jani.

Quoted 198 lines

For what I was debugging I actually wanted to be able to turn on just the DPCD logging without having to turn on all the KMS debugging.. but I can understand the inverse..

BR,
-R

Quoted 190 lines

BR,
Jani.

Quoted 179 lines
---
 drivers/gpu/drm/drm_dp_helper.c | 66 ++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h              |  6 ++++
 2 files changed, 58 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +52 -14
index 291734e..8e17f55 100644
@@ -159,6 +159,46 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
+static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+     size_t ret;
+
+     mutex_lock(&aux->hw_mutex);
+
+     DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
+                     msg->request, msg->address, msg->size);
+     if (unlikely(drm_debug & DRM_UT_DPCD)) {
+             switch (msg->request & ~DP_AUX_I2C_MOT) {
+             case DP_AUX_NATIVE_WRITE:
+             case DP_AUX_I2C_WRITE:
+                     print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+                                     16, 1, msg->buffer, msg->size, false);
+                     break;
+             default:
+                     break;
+             }
+     }
+
+     ret = aux->transfer(aux, msg);
+
+     DRM_DEBUG_DPCD("%s: reply=0x%02x, size=%zu\n", aux->name, msg->reply, ret);
+     if (unlikely(drm_debug & DRM_UT_DPCD)) {
+             switch (msg->request & ~DP_AUX_I2C_MOT) {
+             case DP_AUX_NATIVE_READ:
+             case DP_AUX_I2C_READ:
+                     print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+                                     16, 1, msg->buffer, ret, false);
+                     break;
+             default:
+                     break;
+             }
+     }
+
+     mutex_unlock(&aux->hw_mutex);
+
+     return ret;
+}
+
 #define AUX_RETRY_INTERVAL 500 /* us */
 
 /**
@@ -194,9 +234,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
       */
      for (retry = 0; retry < 32; retry++) {
 
-             mutex_lock(&aux->hw_mutex);
-             err = aux->transfer(aux, &msg);
-             mutex_unlock(&aux->hw_mutex);
+             err = aux_transfer(aux, &msg);
              if (err < 0) {
                      if (err == -EBUSY)
                              continue;
@@ -212,15 +250,17 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
                      return err;
 
              case DP_AUX_NATIVE_REPLY_NACK:
+                     DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
                      return -EIO;
 
              case DP_AUX_NATIVE_REPLY_DEFER:
+                     DRM_DEBUG_DPCD("native defer\n");
                      usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
                      break;
              }
      }
 
-     DRM_DEBUG_KMS("too many retries, giving up\n");
+     DRM_DEBUG_DPCD("too many retries, giving up\n");
      return -EIO;
 }
 
@@ -530,14 +570,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
      int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
 
      for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
-             mutex_lock(&aux->hw_mutex);
-             ret = aux->transfer(aux, msg);
-             mutex_unlock(&aux->hw_mutex);
+             ret = aux_transfer(aux, msg);
              if (ret < 0) {
                      if (ret == -EBUSY)
                              continue;
 
-                     DRM_DEBUG_KMS("transaction failed: %d\n", ret);
+                     DRM_DEBUG_DPCD("transaction failed: %d\n", ret);
                      return ret;
              }
 
@@ -551,11 +589,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
                      break;
 
              case DP_AUX_NATIVE_REPLY_NACK:
-                     DRM_DEBUG_KMS("native nack (result=%d, size=%zu)\n", ret, msg->size);
+                     DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", ret, msg->size);
                      return -EREMOTEIO;
 
              case DP_AUX_NATIVE_REPLY_DEFER:
-                     DRM_DEBUG_KMS("native defer\n");
+                     DRM_DEBUG_DPCD("native defer\n");
                      /*
                       * We could check for I2C bit rate capabilities and if
                       * available adjust this interval. We could also be
@@ -582,12 +620,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
                      return ret;
 
              case DP_AUX_I2C_REPLY_NACK:
-                     DRM_DEBUG_KMS("I2C nack (result=%d, size=%zu\n", ret, msg->size);
+                     DRM_DEBUG_DPCD("I2C nack (result=%d, size=%zu\n", ret, msg->size);
                      aux->i2c_nack_count++;
                      return -EREMOTEIO;
 
              case DP_AUX_I2C_REPLY_DEFER:
-                     DRM_DEBUG_KMS("I2C defer\n");
+                     DRM_DEBUG_DPCD("I2C defer\n");
                      /* DP Compliance Test 4.2.2.5 Requirement:
                       * Must have at least 7 retries for I2C defers on the
                       * transaction to pass this test
@@ -604,7 +642,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
              }
      }
 
-     DRM_DEBUG_KMS("too many retries, giving up\n");
+     DRM_DEBUG_DPCD("too many retries, giving up\n");
      return -EREMOTEIO;
 }
 
@@ -624,7 +662,7 @@ static int drm_dp_i2c_drain_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *o
                      return err == 0 ? -EPROTO : err;
 
              if (err < msg.size && err < ret) {
-                     DRM_DEBUG_KMS("Partial I2C reply: requested %zu bytes got %d bytes\n",
+                     DRM_DEBUG_DPCD("Partial I2C reply: requested %zu bytes got %d bytes\n",
                                    msg.size, err);
                      ret = err;
              }
include/drm/drmP.h +6 -1
index 8b5ce7c..f6c25cc 100644
@@ -125,6 +125,7 @@ struct dma_buf_attachment;
 #define DRM_UT_KMS           0x04
 #define DRM_UT_PRIME         0x08
 #define DRM_UT_ATOMIC                0x10
+#define DRM_UT_DPCD          0x20
 
 extern __printf(2, 3)
 void drm_ut_debug_printk(const char *function_name,
@@ -217,6 +218,11 @@ void drm_err(const char *format, ...);
              if (unlikely(drm_debug & DRM_UT_ATOMIC))                \
                      drm_ut_debug_printk(__func__, fmt, ##args);     \
      } while (0)
+#define DRM_DEBUG_DPCD(fmt, args...)                                 \
+     do {                                                            \
+             if (unlikely(drm_debug & DRM_UT_DPCD))                  \
+                     drm_ut_debug_printk(__func__, fmt, ##args);     \
+     } while (0)
 
 /*@}*/
 
--

2.1.0

Quoted 4 lines

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Jani Nikula, Intel Open Source Technology Center

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#5 lore raw

PATCH 1/1 drm/dp: add DPCD logging

In reply to Rob Clark

Add a new drm_debug bit for turning on DPCD logging, to aid debugging with troublesome monitors.

v2: don't try to hexdump the universe if driver returns -errno, and change the "too many retries" traces to DRM_ERROR()

---
 drivers/gpu/drm/drm_dp_helper.c | 69 ++++++++++++++++++++++++++++++++---------
 include/drm/drmP.h              |  6 ++++
 2 files changed, 60 insertions(+), 15 deletions(-)
drivers/gpu/drm/drm_dp_helper.c +54 -15
index 291734e..ccb1f8a 100644
@@ -159,6 +159,48 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
+static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+	ssize_t ret;
+
+	mutex_lock(&aux->hw_mutex);
+
+	DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
+			msg->request, msg->address, msg->size);
+
+	if (unlikely(drm_debug & DRM_UT_DPCD)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_WRITE:
+		case DP_AUX_I2C_WRITE:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, msg->size, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ret = aux->transfer(aux, msg);
+
+	DRM_DEBUG_DPCD("%s: reply=0x%02x, size=%zd\n", aux->name, msg->reply, ret);
+
+	if (unlikely(drm_debug & DRM_UT_DPCD) && (ret > 0)) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_NATIVE_READ:
+		case DP_AUX_I2C_READ:
+			print_hex_dump(KERN_DEBUG, "DPCD: ", DUMP_PREFIX_OFFSET,
+					16, 1, msg->buffer, ret, false);
+			break;
+		default:
+			break;
+		}
+	}
+
+	mutex_unlock(&aux->hw_mutex);
+
+	return ret;
+}
+
 #define AUX_RETRY_INTERVAL 500 /* us */
 
 /**
@@ -194,9 +236,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 	 */
 	for (retry = 0; retry < 32; retry++) {
 
-		mutex_lock(&aux->hw_mutex);
-		err = aux->transfer(aux, &msg);
-		mutex_unlock(&aux->hw_mutex);
+		err = aux_transfer(aux, &msg);
 		if (err < 0) {
 			if (err == -EBUSY)
 				continue;
@@ -204,7 +244,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 			return err;
 		}
 
-
 		switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) {
 		case DP_AUX_NATIVE_REPLY_ACK:
 			if (err < size)
@@ -212,15 +251,17 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 			return err;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
 			return -EIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
+			DRM_DEBUG_DPCD("native defer\n");
 			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
 			break;
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_ERROR("DPCD: too many retries, giving up!\n");
 	return -EIO;
 }
 
@@ -530,14 +571,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
 
 	for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
-		mutex_lock(&aux->hw_mutex);
-		ret = aux->transfer(aux, msg);
-		mutex_unlock(&aux->hw_mutex);
+		ret = aux_transfer(aux, msg);
 		if (ret < 0) {
 			if (ret == -EBUSY)
 				continue;
 
-			DRM_DEBUG_KMS("transaction failed: %d\n", ret);
+			DRM_DEBUG_DPCD("transaction failed: %d\n", ret);
 			return ret;
 		}
 
@@ -551,11 +590,11 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			break;
 
 		case DP_AUX_NATIVE_REPLY_NACK:
-			DRM_DEBUG_KMS("native nack (result=%d, size=%zu)\n", ret, msg->size);
+			DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", ret, msg->size);
 			return -EREMOTEIO;
 
 		case DP_AUX_NATIVE_REPLY_DEFER:
-			DRM_DEBUG_KMS("native defer\n");
+			DRM_DEBUG_DPCD("native defer\n");
 			/*
 			 * We could check for I2C bit rate capabilities and if
 			 * available adjust this interval. We could also be
@@ -582,12 +621,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return ret;
 
 		case DP_AUX_I2C_REPLY_NACK:
-			DRM_DEBUG_KMS("I2C nack (result=%d, size=%zu\n", ret, msg->size);
+			DRM_DEBUG_DPCD("I2C nack (result=%d, size=%zu\n", ret, msg->size);
 			aux->i2c_nack_count++;
 			return -EREMOTEIO;
 
 		case DP_AUX_I2C_REPLY_DEFER:
-			DRM_DEBUG_KMS("I2C defer\n");
+			DRM_DEBUG_DPCD("I2C defer\n");
 			/* DP Compliance Test 4.2.2.5 Requirement:
 			 * Must have at least 7 retries for I2C defers on the
 			 * transaction to pass this test
@@ -604,7 +643,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 		}
 	}
 
-	DRM_DEBUG_KMS("too many retries, giving up\n");
+	DRM_ERROR("I2C: too many retries, giving up\n");
 	return -EREMOTEIO;
 }
 
@@ -624,7 +663,7 @@ static int drm_dp_i2c_drain_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *o
 			return err == 0 ? -EPROTO : err;
 
 		if (err < msg.size && err < ret) {
-			DRM_DEBUG_KMS("Partial I2C reply: requested %zu bytes got %d bytes\n",
+			DRM_DEBUG_DPCD("Partial I2C reply: requested %zu bytes got %d bytes\n",
 				      msg.size, err);
 			ret = err;
 		}
include/drm/drmP.h +6 -0
index 8b5ce7c..f6c25cc 100644
@@ -125,6 +125,7 @@ struct dma_buf_attachment;
 #define DRM_UT_KMS		0x04
 #define DRM_UT_PRIME		0x08
 #define DRM_UT_ATOMIC		0x10
+#define DRM_UT_DPCD		0x20
 
 extern __printf(2, 3)
 void drm_ut_debug_printk(const char *function_name,
@@ -217,6 +218,11 @@ void drm_err(const char *format, ...);
 		if (unlikely(drm_debug & DRM_UT_ATOMIC))		\
 			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
+#define DRM_DEBUG_DPCD(fmt, args...)					\
+	do {								\
+		if (unlikely(drm_debug & DRM_UT_DPCD))			\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
+	} while (0)
 
 /*@}*/
 
Signature · 6 lines
2.5.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw
In reply to Rob Clark
Quoted 3 lines · Rob Clark

Drop unused drm_atomic and fix comment for drm_debug.

Applied to drm-misc.
-Daniel

Quoted 28 lines
---
 drivers/gpu/drm/drm_drv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/gpu/drm/drm_drv.c +1 -3
index 53d09a1..3a8d598 100644
@@ -37,11 +37,9 @@
 #include "drm_legacy.h"
 #include "drm_internal.h"
 
-unsigned int drm_debug = 0;	/* 1 to enable debug output */
+unsigned int drm_debug = 0;	/* bitmask of DRM_UT_x */
 EXPORT_SYMBOL(drm_debug);
 
-bool drm_atomic = 0;
-
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
Signature · 6 lines
2.1.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel