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 at redhat.com>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1274157
> Cc: stable at vger.kernel.org
> Signed-off-by: Rob Clark <robdclark at gmail.com>
Real world wins.
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC