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 15:42:34 +0200 Message-ID: <20170119134234.GY31595@intel.com> References: <1483082068-5833-1-git-send-email-dhinakaran.pandiyan@intel.com> <20170117174041.GB15796@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id B92396E138 for ; Thu, 19 Jan 2017 13:42:38 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170117174041.GB15796@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 T24gVHVlLCBKYW4gMTcsIDIwMTcgYXQgMDk6NDA6NDJBTSAtMDgwMCwgTWFuYXNpIE5hdmFyZSB3 cm90ZToKPiBJIGhhdmUgdmVyaWZpZWQgdGhpcyBwYXRjaCB3aXRoIHRoZSBsYXRlc3QgZHJtLXRp cCBhbmQgaXQgaXMgYWxzbwo+IGFic29sdXRlbHkgbmVjZXNzYXJ5IHRvIGVuc3VyZSB0aGUgbGlu ayBnZXRzIHByb3Blcmx5IHJldHJhaW5lZAo+IGFmdGVyIGxpbmstc3RhdHVzIGlzIEJBRCBhbmQg YWZ0ZXIgYW5ldyBtb2Rlc2V0IGlzIHRyaWdnZXJlZCBieQo+IHVzZXJzcGFjZS4gV2l0aG91dCB0 aGlzIHBhdGNoLCBzaW5jZSBpbnRlbF9kcF9sb25nX3B1bHNlIGdldHMgY2FsbGVkCj4gaXQgcmVz ZXRzIHRoZSBsaW5rIGZhaWx1cmUgdmFsdWVzIGFuZCBsaW5rIHJldHJhaW5pbmcgZG9lcyBub3Qg aGFwcGVuCj4gYXQgbG93ZXIgbGluayByYXRlcy4KCldoeSBhcmUgeW91IHJlc2V0dGluZyB0aGUg ZmFpbHVyZSB2YWx1ZXMgaW4gdGhpcyBmdW5jdGlvbj8gWW91IHNob3VsZApvbmx5IGRvIHRoYXQg d2hlbiBhIGxvbmcgcHVsc2UgaXMgYWN0dWFsbHkgZGV0ZWN0ZWQgSU1PIChhbmQgeWVzLCAKY2Fs bGluZyB0aGUgZnVuY3Rpb24gaW50ZWxfZHBfbG9uZ19wdWxzZSgpIG5hbWUgaXMgcHJldHR5IG11 Y2ggd3JvbmcKbm93KS4KCj4gCj4gVmlsbGUvSmFuaSBjb3VsZCB5b3UgcGxlYXNlIHJldmlldyB0 aGlzIHBhdGNoPwo+IFRoaSBpcyBkZWZpbml0ZWx5IGEgYnVnIGluIHRoZSBleGlzdGluZyBjb2Rl YmFzZSBhbmQgd2UgbmVlZCB0byBnZXQgdGhpcyBtZXJnZWQKPiBzb29uIHRvIGdldCBpdCBmaXhl ZC4KPiAKPiBSZWdhcmRzCj4gTWFuYXNpCj4gCj4gT24gVGh1LCBEZWMgMjksIDIwMTYgYXQgMTE6 MTQ6MjhQTSAtMDgwMCwgRGhpbmFrYXJhbiBQYW5kaXlhbiB3cm90ZToKPiA+IEZyb206ICJOYXZh cmUsIE1hbmFzaSBEIiA8bWFuYXNpLmQubmF2YXJlQGludGVsLmNvbT4KPiA+IAo+ID4gVGhlIGRl dGVjdF9kb25lIGZsYWcgd2FzIGludHJvZHVjZWQgaW4gdGhlICdjb21taXQgN2QyM2UzYzM3YmIz Cj4gPiAoImRybS9pOTE1OiBDbGVhbmluZyB1cCBpbnRlbF9kcF9ocGRfcHVsc2UiKScgaW4gb3Jk ZXIgdG8gYXZvaWQgbXVsdGlwbGUKPiA+IGRldGVjdHMgb24gaG90cGx1ZyB3aGVyZSBpbnRlbF9k cF9sb25nX3B1bHNlKCkgd2FzIGNhbGxlZCBmcm9tIEhQRCBoYW5kbGVyCj4gPiBhcyB3ZWxsIGFz IGludGVsX2RwX2RldGVjdCgpLiBMYXRlciwgJ2NvbW1pdCAxMDE1ODExNjA5YzAKPiA+ICgiZHJt L2k5MTU6IE1vdmUgbG9uZyBocGQgaGFuZGxpbmcgaW50byB0aGUgaG90cGx1ZyB3b3JrIiknIGRl ZmVycmVkIGxvbmcKPiA+IGhwZCBoYW5kbGluZyB0byBob3RwbHVnIHdvcmsgdG8gYXZvaWQgaGFu ZGxpbmcgaXQgdHdpY2UuIEJ1dCwgcmVzZXR0aW5nIHRoZQo+ID4gZmxhZyBhZnRlciBsb25nIGhw ZCBoYW5kbGluZyBsZWFkcyB0byB0aGUgY29kZSBiZWluZyBleGVjdXRlZCBhZ2FpbiBkdXJpbmcK PiA+IG1vZGUgZW51bWVyYXRpb24uCj4gPiAKPiA+IFNvLCBkbyBub3QgcmVzZXQgdGhlIGRldGVj dF9kb25lIGZsYWcgdG8gRmFsc2UgaW4gaW50ZWxfZHBfZGV0ZWN0KCkuIFRoZQo+ID4gZmxhZyBp cyByZXNldCBpbiBpbnRlbF9kcF9ocGRfcHVsc2UoKSB0byBhbGxvdyBhIGZ1bGwgZGV0ZWN0IGFu ZCBzZXQgd2hlbgo+ID4gdGhlIGhvdHBsdWcgd29yayBkb2VzIGEgZnVsbCBEUENEIGRldGVjdC4g SG93ZXZlciBpZiAtPmRldGVjdCgpIGdldHMgY2FsbGVkCj4gPiBkdXJpbmcgbW9kZSBlbnVtZXJh dGlvbiBhZnRlciBhIERQQ0QgZGV0ZWN0LCByZXR1cm4gdGhlIGNhY2hlZCBjb25uZWN0b3IKPiA+ IHN0YXR1cy4KPiA+IAo+ID4gUmVzZXR0aW5nIHRoZSBmbGFnIGluIHRoZSBlbmNvZGVyJ3MgcmVz ZXQgY2FsbGJhY2sgc2hvdWxkIHRha2UgY2FyZSBvZgo+ID4gaG90cGx1ZyBiZXR3ZWVuIHN1c3Bl bmQvcmVzdW1lLgo+ID4gCj4gPiB2MjoKPiA+IEFsbG93IGZ1bGwgZGV0ZWN0IGFmdGVyIGVuY29k ZXIgcmVzZXQuIChWaWxsZSkKPiA+IFNldCB0aGUgZGV0ZWN0X2RvbmUgZmxhZyBmb3IgY29ubmVj dG9yIGRpc2Nvbm5lY3RlZCBjYXNlIHRvby4gKERLKQo+ID4gQ29tbWl0IG1lc3NhZ2UgY2hhbmdl cy4KPiA+IAo+ID4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiA+IENjOiBWaWxsZSBTeXJq YWxhIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4KPiA+IENjOiBBbmRlciBDb25zZWx2 YW5kZSBPbGl2ZWlyYSA8YW5kZXIuY29uc2VsdmFuLmRlLm9saXZlaXJhQGludGVsLmNvbT4KPiA+ IENjOiBKYW5pIE5pa3VsYSA8amFuaS5uaWt1bGFAbGludXguaW50ZWwuY29tPgo+ID4gRml4ZXM6 IGNvbW1pdCA3ZDIzZTNjMzdiYjMgKCJkcm0vaTkxNTogQ2xlYW5pbmcgdXAgaW50ZWxfZHBfaHBk X3B1bHNlIikKPiA+IFNpZ25lZC1vZmYtYnk6IE1hbmFzaSBOYXZhcmUgPG1hbmFzaS5kLm5hdmFy ZUBpbnRlbC5jb20+Cj4gPiBTaWduZWQtb2ZmLWJ5OiBEaGluYWthcmFuIFBhbmRpeWFuIDxkaGlu YWthcmFuLnBhbmRpeWFuQGludGVsLmNvbT4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX2RwLmMgfCA5ICsrKysrLS0tLQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCA1IGluc2Vy dGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pbnRlbF9kcC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+ ID4gaW5kZXggZmIxMjg5Ni4uNjczMmMxNyAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2Ry bS9pOTE1L2ludGVsX2RwLmMKPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rw LmMKPiA+IEBAIC00NTE2LDcgKzQ1MTYsNiBAQCBpbnRlbF9kcF9sb25nX3B1bHNlKHN0cnVjdCBp bnRlbF9jb25uZWN0b3IgKmludGVsX2Nvbm5lY3RvcikKPiA+ICAJaW50ZWxfZHBfc2V0X2VkaWQo aW50ZWxfZHApOwo+ID4gIAlpZiAoaXNfZWRwKGludGVsX2RwKSB8fCBpbnRlbF9jb25uZWN0b3It PmRldGVjdF9lZGlkKQo+ID4gIAkJc3RhdHVzID0gY29ubmVjdG9yX3N0YXR1c19jb25uZWN0ZWQ7 Cj4gPiAtCWludGVsX2RwLT5kZXRlY3RfZG9uZSA9IHRydWU7Cj4gPiAgCj4gPiAgCS8qIFRyeSB0 byByZWFkIHRoZSBzb3VyY2Ugb2YgdGhlIGludGVycnVwdCAqLwo+ID4gIAlpZiAoaW50ZWxfZHAt PmRwY2RbRFBfRFBDRF9SRVZdID49IDB4MTEgJiYKPiA+IEBAIC00NTUxLDEwICs0NTUwLDEwIEBA IGludGVsX2RwX2RldGVjdChzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yLCBib29sIGZv cmNlKQo+ID4gIAkJICAgICAgY29ubmVjdG9yLT5iYXNlLmlkLCBjb25uZWN0b3ItPm5hbWUpOwo+ ID4gIAo+ID4gIAkvKiBJZiBmdWxsIGRldGVjdCBpcyBub3QgcGVyZm9ybWVkIHlldCwgZG8gYSBm dWxsIGRldGVjdCAqLwo+ID4gLQlpZiAoIWludGVsX2RwLT5kZXRlY3RfZG9uZSkKPiA+ICsJaWYg KCFpbnRlbF9kcC0+ZGV0ZWN0X2RvbmUpIHsKPiA+ICsJCWludGVsX2RwLT5kZXRlY3RfZG9uZSA9 IHRydWU7Cj4gPiAgCQlzdGF0dXMgPSBpbnRlbF9kcF9sb25nX3B1bHNlKGludGVsX2RwLT5hdHRh Y2hlZF9jb25uZWN0b3IpOwo+ID4gLQo+ID4gLQlpbnRlbF9kcC0+ZGV0ZWN0X2RvbmUgPSBmYWxz ZTsKPiA+ICsJfQo+ID4gIAo+ID4gIAlyZXR1cm4gc3RhdHVzOwo+ID4gIH0KPiA+IEBAIC00ODU5 LDYgKzQ4NTgsOCBAQCB2b2lkIGludGVsX2RwX2VuY29kZXJfcmVzZXQoc3RydWN0IGRybV9lbmNv ZGVyICplbmNvZGVyKQo+ID4gIAlpZiAobHNwY29uLT5hY3RpdmUpCj4gPiAgCQlsc3Bjb25fcmVz dW1lKGxzcGNvbik7Cj4gPiAgCj4gPiArCWludGVsX2RwLT5kZXRlY3RfZG9uZSA9IGZhbHNlOwo+ ID4gKwo+ID4gIAlwcHNfbG9jayhpbnRlbF9kcCk7Cj4gPiAgCj4gPiAgCWlmIChJU19WQUxMRVlW SUVXKGRldl9wcml2KSB8fCBJU19DSEVSUllWSUVXKGRldl9wcml2KSkKPiA+IC0tIAo+ID4gMi43 LjQKPiA+IAoKLS0gClZpbGxlIFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRl bC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com ([134.134.136.100]:51289 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752646AbdASNnS (ORCPT ); Thu, 19 Jan 2017 08:43:18 -0500 Date: Thu, 19 Jan 2017 15:42:34 +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: <20170119134234.GY31595@intel.com> References: <1483082068-5833-1-git-send-email-dhinakaran.pandiyan@intel.com> <20170117174041.GB15796@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170117174041.GB15796@intel.com> Sender: stable-owner@vger.kernel.org List-ID: 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). > > 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