On Tue, Oct 13, 2015 at 10:30 AM, Jani Nikula
<jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Tue, 13 Oct 2015, Rob Clark <robdclark@xxxxxxxxx> 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.
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
> BR,
> Jani.
>
>
>>
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
