From: Jani Nikula <jani.nikula@linux.intel.com>
To: Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/dp: add DPCD logging
Date: Tue, 13 Oct 2015 17:30:39 +0300 [thread overview]
Message-ID: <87wpuq3kog.fsf@intel.com> (raw)
In-Reply-To: <1444745609-27304-2-git-send-email-robdclark@gmail.com>
On Tue, 13 Oct 2015, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> drivers/gpu/drm/drm_dp_helper.c | 66 ++++++++++++++++++++++++++++++++---------
> include/drm/drmP.h | 6 ++++
> 2 files changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 291734e..8e17f55 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -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;
> }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 8b5ce7c..f6c25cc 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2015-10-13 14:27 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-13 14:13 [PATCH] drm: misc cleanup Rob Clark
2015-10-13 14:13 ` [PATCH] drm/dp: add DPCD logging Rob Clark
2015-10-13 14:30 ` Jani Nikula [this message]
2015-10-13 14:32 ` Rob Clark
2015-10-13 15:00 ` Jani Nikula
2015-10-16 18:53 ` Rob Clark
2015-10-13 15:50 ` [PATCH] drm: misc cleanup Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wpuq3kog.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=robdclark@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.