From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com ([74.125.82.54]:37785 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751712AbcB2Qpq (ORCPT ); Mon, 29 Feb 2016 11:45:46 -0500 Received: by mail-wm0-f54.google.com with SMTP id p65so53666852wmp.0 for ; Mon, 29 Feb 2016 08:45:45 -0800 (PST) Date: Mon, 29 Feb 2016 17:46:26 +0100 From: Daniel Vetter To: Rob Clark Cc: dri-devel@lists.freedesktop.org, Jani Nikula , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Daniel Vetter , stable@vger.kernel.org, Dave Airlie Subject: Re: [PATCH 1/2] drm/dp: move hw_mutex up the call stack Message-ID: <20160229164626.GN32705@phenom.ffwll.local> References: <1456434906-29600-1-git-send-email-robdclark@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1456434906-29600-1-git-send-email-robdclark@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Feb 25, 2016 at 04:15:05PM -0500, Rob Clark 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 > > 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.. > > v2: rebased to come before DPCD/AUX logging patch for easier backport > to stable branches. > > Reported-by: Dave Wysochanski > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1274157 > Cc: stable@vger.kernel.org > Signed-off-by: Rob Clark Reviewed-by: Daniel Vetter Dave, can you pls pick up? Thanks, Daniel > --- > 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 7d58f59..df64ed1 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -179,7 +179,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; > @@ -187,6 +187,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 > @@ -195,25 +197,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); > @@ -222,7 +223,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; > } > > /** > @@ -544,9 +549,7 @@ 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); > if (ret < 0) { > if (ret == -EBUSY) > continue; > @@ -685,6 +688,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]); > @@ -739,6 +744,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 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch