On Wed, Dec 09, 2015 at 04:20:09PM -0500, Rob Clark wrote: Something small missing here perhaps. The patch subject is not part of the commit message body. > 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 > > Technically, according to people who actually have the DP spec, this > should not be required. In practice, it makes some troublesome Dell > monitor (and perhaps others) work, so probably a case of "It's compliant > if it works with windows" on the hw vendor's part.. > > Reported-by: Dave Wysochanski <dwysocha@xxxxxxxxxx> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1274157 > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> Real world wins. Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/drm_dp_helper.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 9535c5b..76e08dc 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -178,7 +178,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; > @@ -186,6 +186,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 > @@ -194,25 +196,24 @@ 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); > if (err < 0) { > 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: > - return -EIO; > + err = -EIO; > + goto unlock; > > case DP_AUX_NATIVE_REPLY_DEFER: > usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100); > @@ -221,7 +222,11 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, > } > > DRM_DEBUG_KMS("too many retries, giving up\n"); > - return -EIO; > + err = -EIO; > + > +unlock: > + mutex_unlock(&aux->hw_mutex); > + return err; > } > > /** > @@ -542,10 +547,10 @@ 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)); > > + WARN_ON(!mutex_is_locked(&aux->hw_mutex)); > + > 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); > if (ret < 0) { > if (ret == -EBUSY) > continue; > @@ -684,6 +689,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; > drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > @@ -738,6 +745,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 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
