From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm/dp: move hw_mutex up the call stack Date: Fri, 11 Dec 2015 18:32:38 +0200 Message-ID: <20151211163238.GG4437@intel.com> References: <1449696010-27449-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 mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4BF166E0D0 for ; Fri, 11 Dec 2015 08:32:44 -0800 (PST) Content-Disposition: inline In-Reply-To: <1449696010-27449-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: Dave Wysochanski , stable@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBEZWMgMDksIDIwMTUgYXQgMDQ6MjA6MDlQTSAtMDUwMCwgUm9iIENsYXJrIHdyb3Rl OgoKU29tZXRoaW5nIHNtYWxsIG1pc3NpbmcgaGVyZSBwZXJoYXBzLiBUaGUgcGF0Y2ggc3ViamVj dCBpcyBub3QgcGFydApvZiB0aGUgY29tbWl0IG1lc3NhZ2UgYm9keS4KCj4gMSkgZG9uJ3QgbGV0 IG90aGVyIHRocmVhZHMgdHJ5aW5nIHRvIGJhbmcgb24gYXV4IGNoYW5uZWwgaW50ZXJydXB0IHRo ZQo+IGRlZmVyIHRpbWVvdXQvbG9naWMKPiAyKSBkb24ndCBsZXQgb3RoZXIgdGhyZWFkcyBpbnRl cnJ1cHQgdGhlIGkyYyBvdmVyIGF1eCBsb2dpYwo+IAo+IFRlY2huaWNhbGx5LCBhY2NvcmRpbmcg dG8gcGVvcGxlIHdobyBhY3R1YWxseSBoYXZlIHRoZSBEUCBzcGVjLCB0aGlzCj4gc2hvdWxkIG5v dCBiZSByZXF1aXJlZC4gIEluIHByYWN0aWNlLCBpdCBtYWtlcyBzb21lIHRyb3VibGVzb21lIERl bGwKPiBtb25pdG9yIChhbmQgcGVyaGFwcyBvdGhlcnMpIHdvcmssIHNvIHByb2JhYmx5IGEgY2Fz ZSBvZiAiSXQncyBjb21wbGlhbnQKPiBpZiBpdCB3b3JrcyB3aXRoIHdpbmRvd3MiIG9uIHRoZSBo dyB2ZW5kb3IncyBwYXJ0Li4KPiAKPiBSZXBvcnRlZC1ieTogRGF2ZSBXeXNvY2hhbnNraSA8ZHd5 c29jaGFAcmVkaGF0LmNvbT4KPiBCdWd6aWxsYTogaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29t L3Nob3dfYnVnLmNnaT9pZD0xMjc0MTU3Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiBT aWduZWQtb2ZmLWJ5OiBSb2IgQ2xhcmsgPHJvYmRjbGFya0BnbWFpbC5jb20+CgpSZWFsIHdvcmxk IHdpbnMuCgpSZXZpZXdlZC1ieTogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4 LmludGVsLmNvbT4KCj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9kcm1fZHBfaGVscGVyLmMgfCAy OSArKysrKysrKysrKysrKysrKysrLS0tLS0tLS0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMTkgaW5z ZXJ0aW9ucygrKSwgMTAgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1 L2RybS9kcm1fZHBfaGVscGVyLmMgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2RwX2hlbHBlci5jCj4g aW5kZXggOTUzNWM1Yi4uNzZlMDhkYyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vZHJt X2RwX2hlbHBlci5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2RybV9kcF9oZWxwZXIuYwo+IEBA IC0xNzgsNyArMTc4LDcgQEAgc3RhdGljIGludCBkcm1fZHBfZHBjZF9hY2Nlc3Moc3RydWN0IGRy bV9kcF9hdXggKmF1eCwgdTggcmVxdWVzdCwKPiAgewo+ICAJc3RydWN0IGRybV9kcF9hdXhfbXNn IG1zZzsKPiAgCXVuc2lnbmVkIGludCByZXRyeTsKPiAtCWludCBlcnI7Cj4gKwlpbnQgZXJyID0g MDsKPiAgCj4gIAltZW1zZXQoJm1zZywgMCwgc2l6ZW9mKG1zZykpOwo+ICAJbXNnLmFkZHJlc3Mg PSBvZmZzZXQ7Cj4gQEAgLTE4Niw2ICsxODYsOCBAQCBzdGF0aWMgaW50IGRybV9kcF9kcGNkX2Fj Y2VzcyhzdHJ1Y3QgZHJtX2RwX2F1eCAqYXV4LCB1OCByZXF1ZXN0LAo+ICAJbXNnLmJ1ZmZlciA9 IGJ1ZmZlcjsKPiAgCW1zZy5zaXplID0gc2l6ZTsKPiAgCj4gKwltdXRleF9sb2NrKCZhdXgtPmh3 X211dGV4KTsKPiArCj4gIAkvKgo+ICAJICogVGhlIHNwZWNpZmljYXRpb24gZG9lc24ndCBnaXZl IGFueSByZWNvbW1lbmRhdGlvbiBvbiBob3cgb2Z0ZW4gdG8KPiAgCSAqIHJldHJ5IG5hdGl2ZSB0 cmFuc2FjdGlvbnMuIFdlIHVzZWQgdG8gcmV0cnkgNyB0aW1lcyBsaWtlIGZvcgo+IEBAIC0xOTQs MjUgKzE5NiwyNCBAQCBzdGF0aWMgaW50IGRybV9kcF9kcGNkX2FjY2VzcyhzdHJ1Y3QgZHJtX2Rw X2F1eCAqYXV4LCB1OCByZXF1ZXN0LAo+ICAJICovCj4gIAlmb3IgKHJldHJ5ID0gMDsgcmV0cnkg PCAzMjsgcmV0cnkrKykgewo+ICAKPiAtCQltdXRleF9sb2NrKCZhdXgtPmh3X211dGV4KTsKPiAg CQllcnIgPSBhdXgtPnRyYW5zZmVyKGF1eCwgJm1zZyk7Cj4gLQkJbXV0ZXhfdW5sb2NrKCZhdXgt Pmh3X211dGV4KTsKPiAgCQlpZiAoZXJyIDwgMCkgewo+ICAJCQlpZiAoZXJyID09IC1FQlVTWSkK PiAgCQkJCWNvbnRpbnVlOwo+ICAKPiAtCQkJcmV0dXJuIGVycjsKPiArCQkJZ290byB1bmxvY2s7 Cj4gIAkJfQo+ICAKPiAgCj4gIAkJc3dpdGNoIChtc2cucmVwbHkgJiBEUF9BVVhfTkFUSVZFX1JF UExZX01BU0spIHsKPiAgCQljYXNlIERQX0FVWF9OQVRJVkVfUkVQTFlfQUNLOgo+ICAJCQlpZiAo ZXJyIDwgc2l6ZSkKPiAtCQkJCXJldHVybiAtRVBST1RPOwo+IC0JCQlyZXR1cm4gZXJyOwo+ICsJ CQkJZXJyID0gLUVQUk9UTzsKPiArCQkJZ290byB1bmxvY2s7Cj4gIAo+ICAJCWNhc2UgRFBfQVVY X05BVElWRV9SRVBMWV9OQUNLOgo+IC0JCQlyZXR1cm4gLUVJTzsKPiArCQkJZXJyID0gLUVJTzsK PiArCQkJZ290byB1bmxvY2s7Cj4gIAo+ICAJCWNhc2UgRFBfQVVYX05BVElWRV9SRVBMWV9ERUZF UjoKPiAgCQkJdXNsZWVwX3JhbmdlKEFVWF9SRVRSWV9JTlRFUlZBTCwgQVVYX1JFVFJZX0lOVEVS VkFMICsgMTAwKTsKPiBAQCAtMjIxLDcgKzIyMiwxMSBAQCBzdGF0aWMgaW50IGRybV9kcF9kcGNk X2FjY2VzcyhzdHJ1Y3QgZHJtX2RwX2F1eCAqYXV4LCB1OCByZXF1ZXN0LAo+ICAJfQo+ICAKPiAg CURSTV9ERUJVR19LTVMoInRvbyBtYW55IHJldHJpZXMsIGdpdmluZyB1cFxuIik7Cj4gLQlyZXR1 cm4gLUVJTzsKPiArCWVyciA9IC1FSU87Cj4gKwo+ICt1bmxvY2s6Cj4gKwltdXRleF91bmxvY2so JmF1eC0+aHdfbXV0ZXgpOwo+ICsJcmV0dXJuIGVycjsKPiAgfQo+ICAKPiAgLyoqCj4gQEAgLTU0 MiwxMCArNTQ3LDEwIEBAIHN0YXRpYyBpbnQgZHJtX2RwX2kyY19kb19tc2coc3RydWN0IGRybV9k cF9hdXggKmF1eCwgc3RydWN0IGRybV9kcF9hdXhfbXNnICptc2cpCj4gIAkgKi8KPiAgCWludCBt YXhfcmV0cmllcyA9IG1heCg3LCBkcm1fZHBfaTJjX3JldHJ5X2NvdW50KG1zZywgZHBfYXV4X2ky Y19zcGVlZF9raHopKTsKPiAgCj4gKwlXQVJOX09OKCFtdXRleF9pc19sb2NrZWQoJmF1eC0+aHdf bXV0ZXgpKTsKPiArCj4gIAlmb3IgKHJldHJ5ID0gMCwgZGVmZXJfaTJjID0gMDsgcmV0cnkgPCAo bWF4X3JldHJpZXMgKyBkZWZlcl9pMmMpOyByZXRyeSsrKSB7Cj4gLQkJbXV0ZXhfbG9jaygmYXV4 LT5od19tdXRleCk7Cj4gIAkJcmV0ID0gYXV4LT50cmFuc2ZlcihhdXgsIG1zZyk7Cj4gLQkJbXV0 ZXhfdW5sb2NrKCZhdXgtPmh3X211dGV4KTsKPiAgCQlpZiAocmV0IDwgMCkgewo+ICAJCQlpZiAo cmV0ID09IC1FQlVTWSkKPiAgCQkJCWNvbnRpbnVlOwo+IEBAIC02ODQsNiArNjg5LDggQEAgc3Rh dGljIGludCBkcm1fZHBfaTJjX3hmZXIoc3RydWN0IGkyY19hZGFwdGVyICphZGFwdGVyLCBzdHJ1 Y3QgaTJjX21zZyAqbXNncywKPiAgCj4gIAltZW1zZXQoJm1zZywgMCwgc2l6ZW9mKG1zZykpOwo+ ICAKPiArCW11dGV4X2xvY2soJmF1eC0+aHdfbXV0ZXgpOwo+ICsKPiAgCWZvciAoaSA9IDA7IGkg PCBudW07IGkrKykgewo+ICAJCW1zZy5hZGRyZXNzID0gbXNnc1tpXS5hZGRyOwo+ICAJCWRybV9k cF9pMmNfbXNnX3NldF9yZXF1ZXN0KCZtc2csICZtc2dzW2ldKTsKPiBAQCAtNzM4LDYgKzc0NSw4 IEBAIHN0YXRpYyBpbnQgZHJtX2RwX2kyY194ZmVyKHN0cnVjdCBpMmNfYWRhcHRlciAqYWRhcHRl ciwgc3RydWN0IGkyY19tc2cgKm1zZ3MsCj4gIAltc2cuc2l6ZSA9IDA7Cj4gIAkodm9pZClkcm1f ZHBfaTJjX2RvX21zZyhhdXgsICZtc2cpOwo+ICAKPiArCW11dGV4X3VubG9jaygmYXV4LT5od19t dXRleCk7Cj4gKwo+ICAJcmV0dXJuIGVycjsKPiAgfQo+ICAKPiAtLSAKPiAyLjUuMAo+IAo+IF9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gZHJpLWRldmVs IG1haWxpbmcgbGlzdAo+IGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPiBodHRwOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCgotLSAKVmls bGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZv L2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:51728 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752625AbbLKQct (ORCPT ); Fri, 11 Dec 2015 11:32:49 -0500 Date: Fri, 11 Dec 2015 18:32:38 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Rob Clark Cc: dri-devel@lists.freedesktop.org, Dave Wysochanski , stable@vger.kernel.org Subject: Re: [PATCH 1/2] drm/dp: move hw_mutex up the call stack Message-ID: <20151211163238.GG4437@intel.com> References: <1449696010-27449-1-git-send-email-robdclark@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1449696010-27449-1-git-send-email-robdclark@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: 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 > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1274157 > Cc: stable@vger.kernel.org > Signed-off-by: Rob Clark Real world wins. Reviewed-by: Ville Syrj�l� > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj�l� Intel OTC