On Thu, 2016-02-25 at 16:15 -0500, Rob Clark wrote:
> 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()
> v3: rename to more generic "AUX" instead of DP specific DPCD, add
> DP_AUX_I2C_WRITE_STATUS_UPDATE
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
I used the rebased version of this after Ville pointed me to it for
remote debugging some odd adaptor behavior. It'd be quite useful imo, so:
Acked-by: Imre Deak <imre.deak at intel.com>
> ---
> Â drivers/gpu/drm/drm_dp_helper.c | 62
> +++++++++++++++++++++++++++++++++--------
>  include/drm/drmP.h              |  8 +++++-
> Â 2 files changed, 58 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index df64ed1..3ef35fd 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -160,6 +160,45 @@ 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;
> +
> + DRM_DEBUG_AUX("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
> + msg->request, msg->address, msg->size);
> +
> + if (unlikely(drm_debug & DRM_UT_AUX)) {
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_NATIVE_WRITE:
> + case DP_AUX_I2C_WRITE:
> + case DP_AUX_I2C_WRITE_STATUS_UPDATE:
> + 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_AUX("%s: reply=0x%02x, size=%zd\n", aux->name, msg->reply,
> ret);
> +
> + if (unlikely(drm_debug & DRM_UT_AUX) && (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;
> + }
> + }
> +
> + return ret;
> +}
> +
> Â #define AUX_RETRY_INTERVAL 500 /* us */
> Â
> Â /**
> @@ -197,7 +236,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
> Â Â */
> Â for (retry = 0; retry < 32; retry++) {
> Â
> - err = aux->transfer(aux, &msg);
> + err = aux_transfer(aux, &msg);
> Â if (err < 0) {
> Â if (err == -EBUSY)
> Â continue;
> @@ -205,7 +244,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
> Â goto unlock;
> Â }
> Â
> -
> Â switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) {
> Â case DP_AUX_NATIVE_REPLY_ACK:
> Â if (err < size)
> @@ -213,16 +251,18 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux,
> u8 request,
> Â goto unlock;
> Â
> Â case DP_AUX_NATIVE_REPLY_NACK:
> + DRM_DEBUG_AUX("native nack (result=%d, size=%zu)\n",
> err, msg.size);
> Â err = -EIO;
> Â goto unlock;
> Â
> Â case DP_AUX_NATIVE_REPLY_DEFER:
> + DRM_DEBUG_AUX("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");
> Â err = -EIO;
> Â
> Â unlock:
> @@ -549,12 +589,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++) {
> - ret = aux->transfer(aux, msg);
> + ret = aux_transfer(aux, msg);
> Â if (ret < 0) {
> Â if (ret == -EBUSY)
> Â continue;
> Â
> - DRM_DEBUG_KMS("transaction failed: %d\n", ret);
> + DRM_DEBUG_AUX("transaction failed: %d\n", ret);
> Â return ret;
> Â }
> Â
> @@ -568,11 +608,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_AUX("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_AUX("native defer\n");
> Â /*
> Â Â * We could check for I2C bit rate capabilities and if
> Â Â * available adjust this interval. We could also be
> @@ -601,12 +641,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_AUX("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_AUX("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
> @@ -625,7 +665,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;
> Â }
> Â
> @@ -653,7 +693,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_AUX("Partial I2C reply: requested %zu bytes
> got %d bytes\n",
> Â Â Â Â Â Â Â msg.size, err);
> Â ret = err;
> Â }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 3c8422c..cc524b5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -117,7 +117,7 @@ struct dma_buf_attachment;
> Â * drm.debug=0x2 will enable DRIVER messages
> Â * drm.debug=0x3 will enable CORE and DRIVER messages
> Â * ...
> - * drm.debug=0x3f will enable all messages
> + * drm.debug=0x7f will enable all messages
> Â *
> Â * An interesting feature is that it's possible to enable verbose logging at
> Â * run-time by echoing the debug value in its sysfs node:
> @@ -129,6 +129,7 @@ struct dma_buf_attachment;
> Â #define DRM_UT_PRIME 0x08
> Â #define DRM_UT_ATOMIC 0x10
> Â #define DRM_UT_VBL 0x20
> +#define DRM_UT_AUX 0x40
> Â
> Â extern __printf(2, 3)
> Â void drm_ut_debug_printk(const char *function_name,
> @@ -226,6 +227,11 @@ void drm_err(const char *format, ...);
> Â if (unlikely(drm_debug & DRM_UT_VBL)) \
> Â drm_ut_debug_printk(__func__, fmt, ##args); \
> Â } while (0)
> +#define DRM_DEBUG_AUX(fmt, args...) \
> + do { \
> + if (unlikely(drm_debug & DRM_UT_AUX)) \
> + drm_ut_debug_printk(__func__, fmt, ##args); \
> + } while (0)
> Â
> Â /*@}*/
> Â
