[PATCH 2/2] drm/dp: add DPCD/AUX logging
Rob Clark
robdclark at gmail.com
Thu Dec 22 17:11:40 UTC 2016
On Thu, Dec 22, 2016 at 2:02 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Dec 21, 2016 at 05:21:18PM -0500, Rob Clark wrote:
>> On Wed, Dec 21, 2016 at 3:41 PM, Imre Deak <imre.deak at intel.com> wrote:
>> > 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>
>>
>> tbh I completely forgot about this patch.. mind posting the rebased
>> version (or push to a git tree somewhere)? IIRC there were just some
>> minor cosmetic things to fix up, probably worth doing that before I
>> forget about it again..
>
> There's patches for dynamic debugging floating around, I think that'd be
> the more interesting approach than adding ever more magic bits that no one
> understands. But besides that this makes sense.
could make sense.. and by all means if someone wants to take over this
patch and do something more clever, go for it..
I won't have access to a dp monitor for a couple weeks, so other than
minor cosmetic tweaks there isn't much I can do.. and I mostly don't
want this to fall through the cracks again.
BR,
-R
> -Daniel
>
>>
>> BR,
>> -R
>>
>>
>> >
>> >> ---
>> >> 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)
>> >>
>> >> /*@}*/
>> >>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
More information about the dri-devel
mailing list