From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect Date: Thu, 19 Jan 2017 21:26:36 +0200 Message-ID: <20170119192636.GZ31595@intel.com> References: <1483082068-5833-1-git-send-email-dhinakaran.pandiyan@intel.com> <20170117174041.GB15796@intel.com> <20170119134234.GY31595@intel.com> <20170119190101.GA30494@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9A1A56E00D for ; Thu, 19 Jan 2017 19:26:40 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170119190101.GA30494@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Manasi Navare Cc: Ander Conselvande Oliveira , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gVGh1LCBKYW4gMTksIDIwMTcgYXQgMTE6MDE6MDFBTSAtMDgwMCwgTWFuYXNpIE5hdmFyZSB3 cm90ZToKPiBPbiBUaHUsIEphbiAxOSwgMjAxNyBhdCAwMzo0MjozNFBNICswMjAwLCBWaWxsZSBT eXJqw6Rsw6Qgd3JvdGU6Cj4gPiBPbiBUdWUsIEphbiAxNywgMjAxNyBhdCAwOTo0MDo0MkFNIC0w ODAwLCBNYW5hc2kgTmF2YXJlIHdyb3RlOgo+ID4gPiBJIGhhdmUgdmVyaWZpZWQgdGhpcyBwYXRj aCB3aXRoIHRoZSBsYXRlc3QgZHJtLXRpcCBhbmQgaXQgaXMgYWxzbwo+ID4gPiBhYnNvbHV0ZWx5 IG5lY2Vzc2FyeSB0byBlbnN1cmUgdGhlIGxpbmsgZ2V0cyBwcm9wZXJseSByZXRyYWluZWQKPiA+ ID4gYWZ0ZXIgbGluay1zdGF0dXMgaXMgQkFEIGFuZCBhZnRlciBhbmV3IG1vZGVzZXQgaXMgdHJp Z2dlcmVkIGJ5Cj4gPiA+IHVzZXJzcGFjZS4gV2l0aG91dCB0aGlzIHBhdGNoLCBzaW5jZSBpbnRl bF9kcF9sb25nX3B1bHNlIGdldHMgY2FsbGVkCj4gPiA+IGl0IHJlc2V0cyB0aGUgbGluayBmYWls dXJlIHZhbHVlcyBhbmQgbGluayByZXRyYWluaW5nIGRvZXMgbm90IGhhcHBlbgo+ID4gPiBhdCBs b3dlciBsaW5rIHJhdGVzLgo+ID4gCj4gPiBXaHkgYXJlIHlvdSByZXNldHRpbmcgdGhlIGZhaWx1 cmUgdmFsdWVzIGluIHRoaXMgZnVuY3Rpb24/IFlvdSBzaG91bGQKPiA+IG9ubHkgZG8gdGhhdCB3 aGVuIGEgbG9uZyBwdWxzZSBpcyBhY3R1YWxseSBkZXRlY3RlZCBJTU8gKGFuZCB5ZXMsIAo+ID4g Y2FsbGluZyB0aGUgZnVuY3Rpb24gaW50ZWxfZHBfbG9uZ19wdWxzZSgpIG5hbWUgaXMgcHJldHR5 IG11Y2ggd3JvbmcKPiA+IG5vdykuCj4gPgo+IAo+IFllcyB0aGUgdmFsdWVzIGZvciBsaW5rIHJh dGUgbmQgbGFuZSBjb3VudCBnZXQgc2V0IHRvIHRoZSBpbnRlbF9kcF9tYXhfbGlua19idygpCj4g YW5kIGRybV9kcF9tYXhfbGFuZV9jb3VudCgpIGluIGludGVsX2RwX2xvbmdfcHVsc2UoKSBhc3N1 bWluZyB0aGF0IHRoaXMgZnVuY3Rpb24gd291bGQKPiBnZXQgY2FsbGVkIG9uIGhvdHBsdWcgYW5k IHRoYXQgd2UgbmVlZCByZXJlYWQgdGhlIGRwY2QgcmVnaXN0ZXJzLiBTbyBzZXR0aW5nIHRoZXNl IHRvIG1heCB2YWx1ZXMKPiBtZWFuIHRoYXQgd2UgaGF2ZSBsb3N0IHRoZSBpbmZvcm1hdGlvbiBh Ym91dCB0aGUgbGluayByYXRlIGFuZCBsYW5lIGNvdW50IGF0IHdoaWNoIGxpbmsgdHJhaW5pbmcg ZmFpbGVkLgo+IAo+IElmIGRldGVjdF9kb25lIGlzIHJlc2V0IGluIGludGVsX2RwX2RldGVjdCgp IHJpZ2h0IGFmdGVyIGNhbGxpbmcgaW50ZWxfZHBfbG9uZ19wdWxzZSgpIHdoaWNoIGlzIGNsZWFy bHkgYSBidWcKPiB0aGVuIGl0IGNhbGxzIHRoZSBpbnRlbF9kcF9sb25nX3B1bHNlKCkgZHVyaW5n IG1vZGUgZW51bWVyYXRpb24gYmVmb3JlIHRoZSBtb2Rlc2V0IGFuZCBpdCBuZXZlciByZXRyYWlu cyBhdCB0aGUgCj4gbG93ZXIgdmFsdWVzIHNpbmNlIHRoZSBtYXggbGluayByYXRlL2xhbmUgY291 bnQgZ2V0IG92ZXJ3cml0dGVuIGluIGludGVsX2RwX2xvbmdfcHVsc2UoKS4KPiBTbyBpdCBpcyBh YnNvbHV0ZWx5IG5lY2Vzc2FyeSB0byBub3QgcmVzZXQgZGV0ZWN0X2RvbmUgZmxhZyBoZXJlIGlu IGludGVsX2RwX2RldGVjdCgpCgpOby4gV2hhdCB5b3UncmUgYXNraW5nIGZvciBpcyBhIGNoYW5n ZSBpbiBiZWhhdmlvdXIgKHRvIGNhbGwKaW50ZWxfZHBfbG9uZ19wdWxzZSgpIGZyb20gLT5kZXRl Y3QoKSBvbmx5IGlmIGl0IHdhcyBqdXN0IHByZWNlZGVkIGJ5IGFuCmFjdHVhbCBsb25nIHB1bHNl KS4gVGhhdCB3YXMgbmV2ZXIgaG93IHRoaXMgY29kZSB3b3JrZWQuIFRoZSBwb2ludCBvZgp0aGUg ZmxhZyB3YXMgdG8gYXZvaWQgY2FsbGluZyBpdCB0d2ljZSB3aGVuIHByb2Nlc3NpbmcgdGhlIEhQ RCBzaW5jZSBpdAp3YXMgZGlyZWN0bHkgY2FsbGVkIGZyb20gaW50ZWxfZHBfaHBkX3B1bHNlKCkg YW5kIHRoZW4gYWdhaW4gZnJvbQotPmRldGVjdCgpLiBTaW5jZSB3ZSBubyBsb25nZXIgY2FsbCBp dCBkaXJlY3RseSBmcm9tCmludGVsX2RwX2hwZF9wdWxzZSgpIHRoZSBmbGFnIGlzIGluIGZhY3Qg dXNlbGVzcy4KCldoaWxlIHRoZSBjaGFuZ2UgeW91IGFzayBmb3IgbWF5IGJlIGRlc2lyYWJsZSwg aGlzdG9yeSBoYXMgc2hvd24KdGhhdCB0aGUgRFAgY29kZSBpcyB2ZXJ5IGZyYWdpbGUsIHNvIEkg ZG9uJ3QgdGhpbmsgd2Ugc2hvdWxkIG1ha2UKdGhlIGxpbmsgc3RhdGUgcHJvcGVydHkgZGVwZW5k IG9uIHNvbWV0aGluZyB0aGF0IG1heSBuZWVkIHRvIGJlCnJldmVydGVkIGlmIGEgcmVncmVzc2lv biBjcm9wcyB1cC4gQW5kIHNvIEkgdGhpbmsgeW91IHNob3VsZCBqdXN0CmNoYW5nZSB5b3VyIG5l dyBjb2RlIHRvIHdvcmsgd2l0aCB0aGUgZXhpc3Rpbmcgc2NoZW1lLiBXZSBjYW4gcHVzaAp0aGUg ZGV0ZWN0X2RvbmUgb3B0aW1pemF0aW9uIGluIGFmdGVyd2FyZHMgc2luY2Ugd2Ugc2hvdWxkIHRo ZW4gYmUKYWJsZSB0byByZXZlcnQgaXQgd2l0aG91dCBoYXZpbmcgdG8gcmV2ZXJ0IGV2ZXJ5dGhp bmcgcmVsYXRlZCB0bwp0aGUgbGluayBzdGF0dXMgcHJvcGVydHkuCgo+IAo+IE1hbmFzaSAKPiA+ ID4gCj4gPiA+IFZpbGxlL0phbmkgY291bGQgeW91IHBsZWFzZSByZXZpZXcgdGhpcyBwYXRjaD8K PiA+ID4gVGhpIGlzIGRlZmluaXRlbHkgYSBidWcgaW4gdGhlIGV4aXN0aW5nIGNvZGViYXNlIGFu ZCB3ZSBuZWVkIHRvIGdldCB0aGlzIG1lcmdlZAo+ID4gPiBzb29uIHRvIGdldCBpdCBmaXhlZC4K PiA+ID4gCj4gPiA+IFJlZ2FyZHMKPiA+ID4gTWFuYXNpCj4gPiA+IAo+ID4gPiBPbiBUaHUsIERl YyAyOSwgMjAxNiBhdCAxMToxNDoyOFBNIC0wODAwLCBEaGluYWthcmFuIFBhbmRpeWFuIHdyb3Rl Ogo+ID4gPiA+IEZyb206ICJOYXZhcmUsIE1hbmFzaSBEIiA8bWFuYXNpLmQubmF2YXJlQGludGVs LmNvbT4KPiA+ID4gPiAKPiA+ID4gPiBUaGUgZGV0ZWN0X2RvbmUgZmxhZyB3YXMgaW50cm9kdWNl ZCBpbiB0aGUgJ2NvbW1pdCA3ZDIzZTNjMzdiYjMKPiA+ID4gPiAoImRybS9pOTE1OiBDbGVhbmlu ZyB1cCBpbnRlbF9kcF9ocGRfcHVsc2UiKScgaW4gb3JkZXIgdG8gYXZvaWQgbXVsdGlwbGUKPiA+ ID4gPiBkZXRlY3RzIG9uIGhvdHBsdWcgd2hlcmUgaW50ZWxfZHBfbG9uZ19wdWxzZSgpIHdhcyBj YWxsZWQgZnJvbSBIUEQgaGFuZGxlcgo+ID4gPiA+IGFzIHdlbGwgYXMgaW50ZWxfZHBfZGV0ZWN0 KCkuIExhdGVyLCAnY29tbWl0IDEwMTU4MTE2MDljMAo+ID4gPiA+ICgiZHJtL2k5MTU6IE1vdmUg bG9uZyBocGQgaGFuZGxpbmcgaW50byB0aGUgaG90cGx1ZyB3b3JrIiknIGRlZmVycmVkIGxvbmcK PiA+ID4gPiBocGQgaGFuZGxpbmcgdG8gaG90cGx1ZyB3b3JrIHRvIGF2b2lkIGhhbmRsaW5nIGl0 IHR3aWNlLiBCdXQsIHJlc2V0dGluZyB0aGUKPiA+ID4gPiBmbGFnIGFmdGVyIGxvbmcgaHBkIGhh bmRsaW5nIGxlYWRzIHRvIHRoZSBjb2RlIGJlaW5nIGV4ZWN1dGVkIGFnYWluIGR1cmluZwo+ID4g PiA+IG1vZGUgZW51bWVyYXRpb24uCj4gPiA+ID4gCj4gPiA+ID4gU28sIGRvIG5vdCByZXNldCB0 aGUgZGV0ZWN0X2RvbmUgZmxhZyB0byBGYWxzZSBpbiBpbnRlbF9kcF9kZXRlY3QoKS4gVGhlCj4g PiA+ID4gZmxhZyBpcyByZXNldCBpbiBpbnRlbF9kcF9ocGRfcHVsc2UoKSB0byBhbGxvdyBhIGZ1 bGwgZGV0ZWN0IGFuZCBzZXQgd2hlbgo+ID4gPiA+IHRoZSBob3RwbHVnIHdvcmsgZG9lcyBhIGZ1 bGwgRFBDRCBkZXRlY3QuIEhvd2V2ZXIgaWYgLT5kZXRlY3QoKSBnZXRzIGNhbGxlZAo+ID4gPiA+ IGR1cmluZyBtb2RlIGVudW1lcmF0aW9uIGFmdGVyIGEgRFBDRCBkZXRlY3QsIHJldHVybiB0aGUg Y2FjaGVkIGNvbm5lY3Rvcgo+ID4gPiA+IHN0YXR1cy4KPiA+ID4gPiAKPiA+ID4gPiBSZXNldHRp bmcgdGhlIGZsYWcgaW4gdGhlIGVuY29kZXIncyByZXNldCBjYWxsYmFjayBzaG91bGQgdGFrZSBj YXJlIG9mCj4gPiA+ID4gaG90cGx1ZyBiZXR3ZWVuIHN1c3BlbmQvcmVzdW1lLgo+ID4gPiA+IAo+ ID4gPiA+IHYyOgo+ID4gPiA+IEFsbG93IGZ1bGwgZGV0ZWN0IGFmdGVyIGVuY29kZXIgcmVzZXQu IChWaWxsZSkKPiA+ID4gPiBTZXQgdGhlIGRldGVjdF9kb25lIGZsYWcgZm9yIGNvbm5lY3RvciBk aXNjb25uZWN0ZWQgY2FzZSB0b28uIChESykKPiA+ID4gPiBDb21taXQgbWVzc2FnZSBjaGFuZ2Vz Lgo+ID4gPiA+IAo+ID4gPiA+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4gPiA+ID4gQ2M6 IFZpbGxlIFN5cmphbGEgPHZpbGxlLnN5cmphbGFAbGludXguaW50ZWwuY29tPgo+ID4gPiA+IENj OiBBbmRlciBDb25zZWx2YW5kZSBPbGl2ZWlyYSA8YW5kZXIuY29uc2VsdmFuLmRlLm9saXZlaXJh QGludGVsLmNvbT4KPiA+ID4gPiBDYzogSmFuaSBOaWt1bGEgPGphbmkubmlrdWxhQGxpbnV4Lmlu dGVsLmNvbT4KPiA+ID4gPiBGaXhlczogY29tbWl0IDdkMjNlM2MzN2JiMyAoImRybS9pOTE1OiBD bGVhbmluZyB1cCBpbnRlbF9kcF9ocGRfcHVsc2UiKQo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IE1h bmFzaSBOYXZhcmUgPG1hbmFzaS5kLm5hdmFyZUBpbnRlbC5jb20+Cj4gPiA+ID4gU2lnbmVkLW9m Zi1ieTogRGhpbmFrYXJhbiBQYW5kaXlhbiA8ZGhpbmFrYXJhbi5wYW5kaXlhbkBpbnRlbC5jb20+ Cj4gPiA+ID4gLS0tCj4gPiA+ID4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMgfCA5 ICsrKysrLS0tLQo+ID4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCA0IGRl bGV0aW9ucygtKQo+ID4gPiA+IAo+ID4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9kcC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+ID4gPiA+ IGluZGV4IGZiMTI4OTYuLjY3MzJjMTcgMTAwNjQ0Cj4gPiA+ID4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2k5MTUvaW50ZWxfZHAuYwo+ID4gPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2lu dGVsX2RwLmMKPiA+ID4gPiBAQCAtNDUxNiw3ICs0NTE2LDYgQEAgaW50ZWxfZHBfbG9uZ19wdWxz ZShzdHJ1Y3QgaW50ZWxfY29ubmVjdG9yICppbnRlbF9jb25uZWN0b3IpCj4gPiA+ID4gIAlpbnRl bF9kcF9zZXRfZWRpZChpbnRlbF9kcCk7Cj4gPiA+ID4gIAlpZiAoaXNfZWRwKGludGVsX2RwKSB8 fCBpbnRlbF9jb25uZWN0b3ItPmRldGVjdF9lZGlkKQo+ID4gPiA+ICAJCXN0YXR1cyA9IGNvbm5l Y3Rvcl9zdGF0dXNfY29ubmVjdGVkOwo+ID4gPiA+IC0JaW50ZWxfZHAtPmRldGVjdF9kb25lID0g dHJ1ZTsKPiA+ID4gPiAgCj4gPiA+ID4gIAkvKiBUcnkgdG8gcmVhZCB0aGUgc291cmNlIG9mIHRo ZSBpbnRlcnJ1cHQgKi8KPiA+ID4gPiAgCWlmIChpbnRlbF9kcC0+ZHBjZFtEUF9EUENEX1JFVl0g Pj0gMHgxMSAmJgo+ID4gPiA+IEBAIC00NTUxLDEwICs0NTUwLDEwIEBAIGludGVsX2RwX2RldGVj dChzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yLCBib29sIGZvcmNlKQo+ID4gPiA+ICAJ CSAgICAgIGNvbm5lY3Rvci0+YmFzZS5pZCwgY29ubmVjdG9yLT5uYW1lKTsKPiA+ID4gPiAgCj4g PiA+ID4gIAkvKiBJZiBmdWxsIGRldGVjdCBpcyBub3QgcGVyZm9ybWVkIHlldCwgZG8gYSBmdWxs IGRldGVjdCAqLwo+ID4gPiA+IC0JaWYgKCFpbnRlbF9kcC0+ZGV0ZWN0X2RvbmUpCj4gPiA+ID4g KwlpZiAoIWludGVsX2RwLT5kZXRlY3RfZG9uZSkgewo+ID4gPiA+ICsJCWludGVsX2RwLT5kZXRl Y3RfZG9uZSA9IHRydWU7Cj4gPiA+ID4gIAkJc3RhdHVzID0gaW50ZWxfZHBfbG9uZ19wdWxzZShp bnRlbF9kcC0+YXR0YWNoZWRfY29ubmVjdG9yKTsKPiA+ID4gPiAtCj4gPiA+ID4gLQlpbnRlbF9k cC0+ZGV0ZWN0X2RvbmUgPSBmYWxzZTsKPiA+ID4gPiArCX0KPiA+ID4gPiAgCj4gPiA+ID4gIAly ZXR1cm4gc3RhdHVzOwo+ID4gPiAgfQo+ID4gPiA+IEBAIC00ODU5LDYgKzQ4NTgsOCBAQCB2b2lk IGludGVsX2RwX2VuY29kZXJfcmVzZXQoc3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyKQo+ID4g PiA+ICAJaWYgKGxzcGNvbi0+YWN0aXZlKQo+ID4gPiA+ICAJCWxzcGNvbl9yZXN1bWUobHNwY29u KTsKPiA+ID4gPiAgCj4gPiA+ID4gKwlpbnRlbF9kcC0+ZGV0ZWN0X2RvbmUgPSBmYWxzZTsKPiA+ ID4gPiArCj4gPiA+ID4gIAlwcHNfbG9jayhpbnRlbF9kcCk7Cj4gPiA+ID4gIAo+ID4gPiA+ICAJ aWYgKElTX1ZBTExFWVZJRVcoZGV2X3ByaXYpIHx8IElTX0NIRVJSWVZJRVcoZGV2X3ByaXYpKQo+ ID4gPiA+IC0tIAo+ID4gPiA+IDIuNy40Cj4gPiA+ID4gCj4gPiAKPiA+IC0tIAo+ID4gVmlsbGUg U3lyasOkbMOkCj4gPiBJbnRlbCBPVEMKCi0tIApWaWxsZSBTeXJqw6Rsw6QKSW50ZWwgT1RDCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBt YWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3Rz LmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:37625 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751689AbdASTdJ (ORCPT ); Thu, 19 Jan 2017 14:33:09 -0500 Date: Thu, 19 Jan 2017 21:26:36 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Manasi Navare Cc: jani.nikula@linux.intel.com, intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, Ander Conselvande Oliveira Subject: Re: [PATCH v3] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect Message-ID: <20170119192636.GZ31595@intel.com> References: <1483082068-5833-1-git-send-email-dhinakaran.pandiyan@intel.com> <20170117174041.GB15796@intel.com> <20170119134234.GY31595@intel.com> <20170119190101.GA30494@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170119190101.GA30494@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Jan 19, 2017 at 11:01:01AM -0800, Manasi Navare wrote: > On Thu, Jan 19, 2017 at 03:42:34PM +0200, Ville Syrj�l� wrote: > > On Tue, Jan 17, 2017 at 09:40:42AM -0800, Manasi Navare wrote: > > > I have verified this patch with the latest drm-tip and it is also > > > absolutely necessary to ensure the link gets properly retrained > > > after link-status is BAD and after anew modeset is triggered by > > > userspace. Without this patch, since intel_dp_long_pulse gets called > > > it resets the link failure values and link retraining does not happen > > > at lower link rates. > > > > Why are you resetting the failure values in this function? You should > > only do that when a long pulse is actually detected IMO (and yes, > > calling the function intel_dp_long_pulse() name is pretty much wrong > > now). > > > > Yes the values for link rate nd lane count get set to the intel_dp_max_link_bw() > and drm_dp_max_lane_count() in intel_dp_long_pulse() assuming that this function would > get called on hotplug and that we need reread the dpcd registers. So setting these to max values > mean that we have lost the information about the link rate and lane count at which link training failed. > > If detect_done is reset in intel_dp_detect() right after calling intel_dp_long_pulse() which is clearly a bug > then it calls the intel_dp_long_pulse() during mode enumeration before the modeset and it never retrains at the > lower values since the max link rate/lane count get overwritten in intel_dp_long_pulse(). > So it is absolutely necessary to not reset detect_done flag here in intel_dp_detect() No. What you're asking for is a change in behaviour (to call intel_dp_long_pulse() from ->detect() only if it was just preceded by an actual long pulse). That was never how this code worked. The point of the flag was to avoid calling it twice when processing the HPD since it was directly called from intel_dp_hpd_pulse() and then again from ->detect(). Since we no longer call it directly from intel_dp_hpd_pulse() the flag is in fact useless. While the change you ask for may be desirable, history has shown that the DP code is very fragile, so I don't think we should make the link state property depend on something that may need to be reverted if a regression crops up. And so I think you should just change your new code to work with the existing scheme. We can push the detect_done optimization in afterwards since we should then be able to revert it without having to revert everything related to the link status property. > > Manasi > > > > > > Ville/Jani could you please review this patch? > > > Thi is definitely a bug in the existing codebase and we need to get this merged > > > soon to get it fixed. > > > > > > Regards > > > Manasi > > > > > > On Thu, Dec 29, 2016 at 11:14:28PM -0800, Dhinakaran Pandiyan wrote: > > > > From: "Navare, Manasi D" > > > > > > > > The detect_done flag was introduced in the 'commit 7d23e3c37bb3 > > > > ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple > > > > detects on hotplug where intel_dp_long_pulse() was called from HPD handler > > > > as well as intel_dp_detect(). Later, 'commit 1015811609c0 > > > > ("drm/i915: Move long hpd handling into the hotplug work")' deferred long > > > > hpd handling to hotplug work to avoid handling it twice. But, resetting the > > > > flag after long hpd handling leads to the code being executed again during > > > > mode enumeration. > > > > > > > > So, do not reset the detect_done flag to False in intel_dp_detect(). The > > > > flag is reset in intel_dp_hpd_pulse() to allow a full detect and set when > > > > the hotplug work does a full DPCD detect. However if ->detect() gets called > > > > during mode enumeration after a DPCD detect, return the cached connector > > > > status. > > > > > > > > Resetting the flag in the encoder's reset callback should take care of > > > > hotplug between suspend/resume. > > > > > > > > v2: > > > > Allow full detect after encoder reset. (Ville) > > > > Set the detect_done flag for connector disconnected case too. (DK) > > > > Commit message changes. > > > > > > > > Cc: stable@vger.kernel.org > > > > Cc: Ville Syrjala > > > > Cc: Ander Conselvande Oliveira > > > > Cc: Jani Nikula > > > > Fixes: commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") > > > > Signed-off-by: Manasi Navare > > > > Signed-off-by: Dhinakaran Pandiyan > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- > > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index fb12896..6732c17 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -4516,7 +4516,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > > > > intel_dp_set_edid(intel_dp); > > > > if (is_edp(intel_dp) || intel_connector->detect_edid) > > > > status = connector_status_connected; > > > > - intel_dp->detect_done = true; > > > > > > > > /* Try to read the source of the interrupt */ > > > > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > > > > @@ -4551,10 +4550,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > > > connector->base.id, connector->name); > > > > > > > > /* If full detect is not performed yet, do a full detect */ > > > > - if (!intel_dp->detect_done) > > > > + if (!intel_dp->detect_done) { > > > > + intel_dp->detect_done = true; > > > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > > > - > > > > - intel_dp->detect_done = false; > > > > + } > > > > > > > > return status; > > > } > > > > @@ -4859,6 +4858,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) > > > > if (lspcon->active) > > > > lspcon_resume(lspcon); > > > > > > > > + intel_dp->detect_done = false; > > > > + > > > > pps_lock(intel_dp); > > > > > > > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > > > -- > > > > 2.7.4 > > > > > > > > -- > > Ville Syrj�l� > > Intel OTC -- Ville Syrj�l� Intel OTC