On Wed, 09 Dec 2015, Rob Clark <robdclark@xxxxxxxxx> wrote:
> On Fri, Oct 16, 2015 at 2:54 PM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>> 1) don't let other threads trying to bang on aux channel interrupt the
>> defer timeout/logic
>> 2) don't let other threads interrupt the i2c over aux logic
>>
>> ---
>> This wasn't actually fixing things w/ problematic monitor, but seems
>> like generally a good idea. At least AFAIU you shouldn't allow the
>> sequence of transfers for i2c over aux be interrupted but unrelated
>> chatter.
>
> So, I got confirmation today that this patch actually does fix things
> w/ a different problematic Dell monitor. So I think it actually is
> the right thing to do.
Looking at drm_dp_dpcd_access and drm_dp_i2c_do_msg it seems clear to me
that not holding the mutex will screw up the delays in native and
i2c-over-aux defer handling by letting another caller on the aux.
However this is missing some aux->transfer to aux_transfer abstraction
prep patch; has it been sent to the list? With that addressed, r-b
follows.
BR,
Jani.
>
> BR,
> -R
>
>> This applies on top of the DPCD logging patch.
>>
>> drivers/gpu/drm/drm_dp_helper.c | 27 +++++++++++++++++----------
>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index ccb1f8a..e409b5f 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -163,8 +163,6 @@ static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>> {
>> ssize_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);
>>
>> @@ -196,8 +194,6 @@ static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>> }
>> }
>>
>> - mutex_unlock(&aux->hw_mutex);
>> -
>> return ret;
>> }
>>
>> @@ -220,7 +216,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>> {
>> struct drm_dp_aux_msg msg;
>> unsigned int retry;
>> - int err;
>> + int err = 0;
>>
>> memset(&msg, 0, sizeof(msg));
>> msg.address = offset;
>> @@ -228,6 +224,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>> msg.buffer = buffer;
>> msg.size = size;
>>
>> + mutex_lock(&aux->hw_mutex);
>> +
>> /*
>> * The specification doesn't give any recommendation on how often to
>> * retry native transactions. We used to retry 7 times like for
>> @@ -241,18 +239,19 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>> if (err == -EBUSY)
>> continue;
>>
>> - return err;
>> + goto unlock;
>> }
>>
>> switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) {
>> case DP_AUX_NATIVE_REPLY_ACK:
>> if (err < size)
>> - return -EPROTO;
>> - return err;
>> + err = -EPROTO;
>> + goto unlock;
>>
>> case DP_AUX_NATIVE_REPLY_NACK:
>> DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
>> - return -EIO;
>> + err = -EIO;
>> + goto unlock;
>>
>> case DP_AUX_NATIVE_REPLY_DEFER:
>> DRM_DEBUG_DPCD("native defer\n");
>> @@ -262,7 +261,11 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>> }
>>
>> DRM_ERROR("DPCD: too many retries, giving up!\n");
>> - return -EIO;
>> + err = -EIO;
>> +
>> +unlock:
>> + mutex_unlock(&aux->hw_mutex);
>> + return err;
>> }
>>
>> /**
>> @@ -698,6 +701,8 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>>
>> memset(&msg, 0, sizeof(msg));
>>
>> + mutex_lock(&aux->hw_mutex);
>> +
>> for (i = 0; i < num; i++) {
>> msg.address = msgs[i].addr;
>> msg.request = (msgs[i].flags & I2C_M_RD) ?
>> @@ -741,6 +746,8 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>> msg.size = 0;
>> (void)drm_dp_i2c_do_msg(aux, &msg);
>>
>> + mutex_unlock(&aux->hw_mutex);
>> +
>> return err;
>> }
>>
>> --
>> 2.5.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
