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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] drm/dp: move hw_mutex up the call stack Date: Mon, 29 Feb 2016 17:46:26 +0100 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="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 737806E321 for ; Mon, 29 Feb 2016 16:45:46 +0000 (UTC) Received: by mail-wm0-x22e.google.com with SMTP id l68so66761137wml.1 for ; Mon, 29 Feb 2016 08:45:46 -0800 (PST) Content-Disposition: inline In-Reply-To: <1456434906-29600-1-git-send-email-robdclark@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: dri-devel@lists.freedesktop.org, stable@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBGZWIgMjUsIDIwMTYgYXQgMDQ6MTU6MDVQTSAtMDUwMCwgUm9iIENsYXJrIHdyb3Rl Ogo+IDEpIGRvbid0IGxldCBvdGhlciB0aHJlYWRzIHRyeWluZyB0byBiYW5nIG9uIGF1eCBjaGFu bmVsIGludGVycnVwdCB0aGUKPiBkZWZlciB0aW1lb3V0L2xvZ2ljCj4gMikgZG9uJ3QgbGV0IG90 aGVyIHRocmVhZHMgaW50ZXJydXB0IHRoZSBpMmMgb3ZlciBhdXggbG9naWMKPiAKPiBUZWNobmlj YWxseSwgYWNjb3JkaW5nIHRvIHBlb3BsZSB3aG8gYWN0dWFsbHkgaGF2ZSB0aGUgRFAgc3BlYywg dGhpcwo+IHNob3VsZCBub3QgYmUgcmVxdWlyZWQuICBJbiBwcmFjdGljZSwgaXQgbWFrZXMgc29t ZSB0cm91Ymxlc29tZSBEZWxsCj4gbW9uaXRvciAoYW5kIHBlcmhhcHMgb3RoZXJzKSB3b3JrLCBz byBwcm9iYWJseSBhIGNhc2Ugb2YgIkl0J3MgY29tcGxpYW50Cj4gaWYgaXQgd29ya3Mgd2l0aCB3 aW5kb3dzIiBvbiB0aGUgaHcgdmVuZG9yJ3MgcGFydC4uCj4gCj4gdjI6IHJlYmFzZWQgdG8gY29t ZSBiZWZvcmUgRFBDRC9BVVggbG9nZ2luZyBwYXRjaCBmb3IgZWFzaWVyIGJhY2twb3J0Cj4gdG8g c3RhYmxlIGJyYW5jaGVzLgo+IAo+IFJlcG9ydGVkLWJ5OiBEYXZlIFd5c29jaGFuc2tpIDxkd3lz b2NoYUByZWRoYXQuY29tPgo+IEJ1Z3ppbGxhOiBodHRwczovL2J1Z3ppbGxhLnJlZGhhdC5jb20v c2hvd19idWcuY2dpP2lkPTEyNzQxNTcKPiBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+IFNp Z25lZC1vZmYtYnk6IFJvYiBDbGFyayA8cm9iZGNsYXJrQGdtYWlsLmNvbT4KClJldmlld2VkLWJ5 OiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xsLmNoPgoKRGF2ZSwgY2FuIHlvdSBw bHMgcGljayB1cD8KClRoYW5rcywgRGFuaWVsCgo+IC0tLQo+ICBkcml2ZXJzL2dwdS9kcm0vZHJt X2RwX2hlbHBlci5jIHwgMjcgKysrKysrKysrKysrKysrKystLS0tLS0tLS0tCj4gIDEgZmlsZSBj aGFuZ2VkLCAxNyBpbnNlcnRpb25zKCspLCAxMCBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0 IGEvZHJpdmVycy9ncHUvZHJtL2RybV9kcF9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1f ZHBfaGVscGVyLmMKPiBpbmRleCA3ZDU4ZjU5Li5kZjY0ZWQxIDEwMDY0NAo+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9kcm1fZHBfaGVscGVyLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2Rw X2hlbHBlci5jCj4gQEAgLTE3OSw3ICsxNzksNyBAQCBzdGF0aWMgaW50IGRybV9kcF9kcGNkX2Fj Y2VzcyhzdHJ1Y3QgZHJtX2RwX2F1eCAqYXV4LCB1OCByZXF1ZXN0LAo+ICB7Cj4gIAlzdHJ1Y3Qg ZHJtX2RwX2F1eF9tc2cgbXNnOwo+ICAJdW5zaWduZWQgaW50IHJldHJ5Owo+IC0JaW50IGVycjsK PiArCWludCBlcnIgPSAwOwo+ICAKPiAgCW1lbXNldCgmbXNnLCAwLCBzaXplb2YobXNnKSk7Cj4g IAltc2cuYWRkcmVzcyA9IG9mZnNldDsKPiBAQCAtMTg3LDYgKzE4Nyw4IEBAIHN0YXRpYyBpbnQg ZHJtX2RwX2RwY2RfYWNjZXNzKHN0cnVjdCBkcm1fZHBfYXV4ICphdXgsIHU4IHJlcXVlc3QsCj4g IAltc2cuYnVmZmVyID0gYnVmZmVyOwo+ICAJbXNnLnNpemUgPSBzaXplOwo+ICAKPiArCW11dGV4 X2xvY2soJmF1eC0+aHdfbXV0ZXgpOwo+ICsKPiAgCS8qCj4gIAkgKiBUaGUgc3BlY2lmaWNhdGlv biBkb2Vzbid0IGdpdmUgYW55IHJlY29tbWVuZGF0aW9uIG9uIGhvdyBvZnRlbiB0bwo+ICAJICog cmV0cnkgbmF0aXZlIHRyYW5zYWN0aW9ucy4gV2UgdXNlZCB0byByZXRyeSA3IHRpbWVzIGxpa2Ug Zm9yCj4gQEAgLTE5NSwyNSArMTk3LDI0IEBAIHN0YXRpYyBpbnQgZHJtX2RwX2RwY2RfYWNjZXNz KHN0cnVjdCBkcm1fZHBfYXV4ICphdXgsIHU4IHJlcXVlc3QsCj4gIAkgKi8KPiAgCWZvciAocmV0 cnkgPSAwOyByZXRyeSA8IDMyOyByZXRyeSsrKSB7Cj4gIAo+IC0JCW11dGV4X2xvY2soJmF1eC0+ aHdfbXV0ZXgpOwo+ICAJCWVyciA9IGF1eC0+dHJhbnNmZXIoYXV4LCAmbXNnKTsKPiAtCQltdXRl eF91bmxvY2soJmF1eC0+aHdfbXV0ZXgpOwo+ICAJCWlmIChlcnIgPCAwKSB7Cj4gIAkJCWlmIChl cnIgPT0gLUVCVVNZKQo+ICAJCQkJY29udGludWU7Cj4gIAo+IC0JCQlyZXR1cm4gZXJyOwo+ICsJ CQlnb3RvIHVubG9jazsKPiAgCQl9Cj4gIAo+ICAKPiAgCQlzd2l0Y2ggKG1zZy5yZXBseSAmIERQ X0FVWF9OQVRJVkVfUkVQTFlfTUFTSykgewo+ICAJCWNhc2UgRFBfQVVYX05BVElWRV9SRVBMWV9B Q0s6Cj4gIAkJCWlmIChlcnIgPCBzaXplKQo+IC0JCQkJcmV0dXJuIC1FUFJPVE87Cj4gLQkJCXJl dHVybiBlcnI7Cj4gKwkJCQllcnIgPSAtRVBST1RPOwo+ICsJCQlnb3RvIHVubG9jazsKPiAgCj4g IAkJY2FzZSBEUF9BVVhfTkFUSVZFX1JFUExZX05BQ0s6Cj4gLQkJCXJldHVybiAtRUlPOwo+ICsJ CQllcnIgPSAtRUlPOwo+ICsJCQlnb3RvIHVubG9jazsKPiAgCj4gIAkJY2FzZSBEUF9BVVhfTkFU SVZFX1JFUExZX0RFRkVSOgo+ICAJCQl1c2xlZXBfcmFuZ2UoQVVYX1JFVFJZX0lOVEVSVkFMLCBB VVhfUkVUUllfSU5URVJWQUwgKyAxMDApOwo+IEBAIC0yMjIsNyArMjIzLDExIEBAIHN0YXRpYyBp bnQgZHJtX2RwX2RwY2RfYWNjZXNzKHN0cnVjdCBkcm1fZHBfYXV4ICphdXgsIHU4IHJlcXVlc3Qs Cj4gIAl9Cj4gIAo+ICAJRFJNX0RFQlVHX0tNUygidG9vIG1hbnkgcmV0cmllcywgZ2l2aW5nIHVw XG4iKTsKPiAtCXJldHVybiAtRUlPOwo+ICsJZXJyID0gLUVJTzsKPiArCj4gK3VubG9jazoKPiAr CW11dGV4X3VubG9jaygmYXV4LT5od19tdXRleCk7Cj4gKwlyZXR1cm4gZXJyOwo+ICB9Cj4gIAo+ ICAvKioKPiBAQCAtNTQ0LDkgKzU0OSw3IEBAIHN0YXRpYyBpbnQgZHJtX2RwX2kyY19kb19tc2co c3RydWN0IGRybV9kcF9hdXggKmF1eCwgc3RydWN0IGRybV9kcF9hdXhfbXNnICptc2cpCj4gIAlp bnQgbWF4X3JldHJpZXMgPSBtYXgoNywgZHJtX2RwX2kyY19yZXRyeV9jb3VudChtc2csIGRwX2F1 eF9pMmNfc3BlZWRfa2h6KSk7Cj4gIAo+ICAJZm9yIChyZXRyeSA9IDAsIGRlZmVyX2kyYyA9IDA7 IHJldHJ5IDwgKG1heF9yZXRyaWVzICsgZGVmZXJfaTJjKTsgcmV0cnkrKykgewo+IC0JCW11dGV4 X2xvY2soJmF1eC0+aHdfbXV0ZXgpOwo+ICAJCXJldCA9IGF1eC0+dHJhbnNmZXIoYXV4LCBtc2cp Owo+IC0JCW11dGV4X3VubG9jaygmYXV4LT5od19tdXRleCk7Cj4gIAkJaWYgKHJldCA8IDApIHsK PiAgCQkJaWYgKHJldCA9PSAtRUJVU1kpCj4gIAkJCQljb250aW51ZTsKPiBAQCAtNjg1LDYgKzY4 OCw4IEBAIHN0YXRpYyBpbnQgZHJtX2RwX2kyY194ZmVyKHN0cnVjdCBpMmNfYWRhcHRlciAqYWRh cHRlciwgc3RydWN0IGkyY19tc2cgKm1zZ3MsCj4gIAo+ICAJbWVtc2V0KCZtc2csIDAsIHNpemVv Zihtc2cpKTsKPiAgCj4gKwltdXRleF9sb2NrKCZhdXgtPmh3X211dGV4KTsKPiArCj4gIAlmb3Ig KGkgPSAwOyBpIDwgbnVtOyBpKyspIHsKPiAgCQltc2cuYWRkcmVzcyA9IG1zZ3NbaV0uYWRkcjsK PiAgCQlkcm1fZHBfaTJjX21zZ19zZXRfcmVxdWVzdCgmbXNnLCAmbXNnc1tpXSk7Cj4gQEAgLTcz OSw2ICs3NDQsOCBAQCBzdGF0aWMgaW50IGRybV9kcF9pMmNfeGZlcihzdHJ1Y3QgaTJjX2FkYXB0 ZXIgKmFkYXB0ZXIsIHN0cnVjdCBpMmNfbXNnICptc2dzLAo+ICAJbXNnLnNpemUgPSAwOwo+ICAJ KHZvaWQpZHJtX2RwX2kyY19kb19tc2coYXV4LCAmbXNnKTsKPiAgCj4gKwltdXRleF91bmxvY2so JmF1eC0+aHdfbXV0ZXgpOwo+ICsKPiAgCXJldHVybiBlcnI7Cj4gIH0KPiAgCj4gLS0gCj4gMi41 LjAKPiAKCi0tIApEYW5pZWwgVmV0dGVyClNvZnR3YXJlIEVuZ2luZWVyLCBJbnRlbCBDb3Jwb3Jh dGlvbgpodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5m cmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0 aW5mby9kcmktZGV2ZWwK