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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 21, 2016 at 3:41 PM, Imre Deak <imre.deak@xxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxx>

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..

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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux