From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Perform link quality check unconditionally during long pulse Date: Thu, 16 Feb 2017 19:18:57 +0200 Message-ID: <20170216171857.GI31595@intel.com> References: <20170216152659.GD31595@intel.com> <20170216153007.14868-1-ville.syrjala@linux.intel.com> <20170216170753.GA27437@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 2F8476EBD8 for ; Thu, 16 Feb 2017 17:19:04 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170216170753.GA27437@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: intel-gfx@lists.freedesktop.org, Palmer Dabbelt , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gVGh1LCBGZWIgMTYsIDIwMTcgYXQgMDk6MDc6NTNBTSAtMDgwMCwgTWFuYXNpIE5hdmFyZSB3 cm90ZToKPiBPbiBUaHUsIEZlYiAxNiwgMjAxNyBhdCAwNTozMDowN1BNICswMjAwLCB2aWxsZS5z eXJqYWxhQGxpbnV4LmludGVsLmNvbSB3cm90ZToKPiA+IEZyb206IFZpbGxlIFN5cmrDpGzDpCA8 dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gPiAKPiA+IEFwcGFyZW50bHkgc29tZSBE UCBzaW5rcyBhcmUgYSBsaXR0bGUgbnV0cyBhbmQgY2F1c2UgSFBEIHRvIGRyb3AKPiA+IGludGVy bWl0dGVudGx5IGR1cmluZyBtb2Rlc2V0cy4gVGhpcyBoYXBwZW5zIGVnLiBvbiBhbiBBU1VTIFBC Mjg3US4KPiA+IEluIG9kZXIgdG8gcmVjb3ZlciBmcm9tIHRoaXMgd2UgY2FuJ3QgcmVhbGx5IHVz ZSB0aGUgcHJldmlvdXMKPiA+IGNvbm5lY3RvciBzdGF0dXMgdG8gZGV0ZXJtaW5lIGlmIHRoZSBs aW5rIG5lZWRzIHJldHJhaW5pbmcsIHNvIGxldCdzCj4gPiBqdXN0IGlnbm9yZSB0aGF0IHBpZWNl IG9mIGluZm9ybWF0aW9uIGFuZCBkbyB0aGUgcmV0cmFpbgo+ID4gdW5jb25kaXRpb25hbGx5LiBX ZSBkbyBvZiBjb3Vyc2Ugc3RpbGwgY2hlY2sgd2hldGhlciB0aGUgbGluayBpcwo+ID4gc3VwcG9z ZWQgdG8gYmUgcnVubmluZyBvciBub3QuCj4gPiAKPiA+IENjOiBzdGFibGVAdmdlci5rZXJuZWwu b3JnCj4gPiBDYzogUGFsbWVyIERhYmJlbHQgPHBhbG1lckBkYWJiZWx0LmNvbT4KPiA+IFJlcG9y dGVkLWJ5OiBQYWxtZXIgRGFiYmVsdCA8cGFsbWVyQGRhYmJlbHQuY29tPgo+ID4gUmVmZXJlbmNl czogaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvYXJjaGl2ZXMvaW50ZWwtZ2Z4LzIwMTct RmVicnVhcnkvMTE5Nzc5Lmh0bWwKPiA+IFNpZ25lZC1vZmYtYnk6IFZpbGxlIFN5cmrDpGzDpCA8 dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2dwdS9k cm0vaTkxNS9pbnRlbF9kcC5jIHwgMTUgKysrKysrKysrKystLS0tCj4gPiAgMSBmaWxlIGNoYW5n ZWQsIDExIGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQg YS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfZHAuYwo+ID4gaW5kZXggMDI0Nzk4YTljMDE2Li4zN2E3NDZmN2ZiYzMgMTAwNjQ0Cj4g PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcC5jCj4gPiArKysgYi9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pbnRlbF9kcC5jCj4gPiBAQCAtNDY0OCwxMSArNDY0OCwxOCBAQCBpbnRl bF9kcF9sb25nX3B1bHNlKHN0cnVjdCBpbnRlbF9jb25uZWN0b3IgKmludGVsX2Nvbm5lY3RvcikK PiA+ICAJCSAqLwo+ID4gIAkJc3RhdHVzID0gY29ubmVjdG9yX3N0YXR1c19kaXNjb25uZWN0ZWQ7 Cj4gPiAgCQlnb3RvIG91dDsKPiA+IC0JfSBlbHNlIGlmIChjb25uZWN0b3ItPnN0YXR1cyA9PSBj b25uZWN0b3Jfc3RhdHVzX2Nvbm5lY3RlZCkgewo+ID4gKwl9IGVsc2Ugewo+ID4gIAkJLyoKPiA+ IC0JCSAqIElmIGRpc3BsYXkgd2FzIGNvbm5lY3RlZCBhbHJlYWR5IGFuZCBpcyBzdGlsbCBjb25u ZWN0ZWQKPiA+IC0JCSAqIGNoZWNrIGxpbmtzIHN0YXR1cywgdGhlcmUgaGFzIGJlZW4ga25vd24g aXNzdWVzIG9mCj4gPiAtCQkgKiBsaW5rIGxvc3MgdHJpZ2dlcnJpbmcgbG9uZyBwdWxzZSEhISEK PiA+ICsJCSAqIElmIGRpc3BsYXkgaXMgbm93IGNvbm5lY3RlZCBjaGVjayBsaW5rcyBzdGF0dXMs Cj4gPiArCQkgKiB0aGVyZSBoYXMgYmVlbiBrbm93biBpc3N1ZXMgb2YgbGluayBsb3NzIHRyaWdn ZXJyaW5nCj4gPiArCQkgKiBsb25nIHB1bHNlLgo+ID4gKwkJICoKPiA+ICsJCSAqIFNvbWUgc2lu a3MgKGVnLiBBU1VTIFBCMjg3USkgc2VlbSB0byBwZXJmb3JtIHNvbWUKPiA+ICsJCSAqIHdlaXJk IEhQRCBwaW5nIHBvbmcgZHVyaW5nIG1vZGVzZXRzLiBTbyB3ZSBjYW4gYXBwYXJlbHkKPiA+ICsJ CSAqIGVuZCB1cCB3aXRoIEhQRCBnb2luZyBsb3cgZHVyaW5nIGEgbW9kZXNldCwgYW5kIHRoZW4K PiA+ICsJCSAqIGdvaW5nIGJhY2sgdXAgc29vbiBhZnRlci4gQW5kIG9uY2UgdGhhdCBoYXBwZW5z IHdlIG11c3QKPiA+ICsJCSAqIHJldHJhaW4gdGhlIGxpbmsgdG8gZ2V0IGEgcGljdHVyZS4gVGhh dCdzIGluIGNhc2Ugbm8KPiA+ICsJCSAqIHVzZXJzcGFjZSBjb21wb25lbnQgcmVhY3RlZCB0byBp bnRlcm1pdHRlbnQgSFBEIGRpcC4KPiA+ICAJCSAqLwo+ID4gIAkJZHJtX21vZGVzZXRfbG9jaygm ZGV2LT5tb2RlX2NvbmZpZy5jb25uZWN0aW9uX211dGV4LCBOVUxMKTsKPiA+ICAJCWludGVsX2Rw X2NoZWNrX2xpbmtfc3RhdHVzKGludGVsX2RwKTsKPiA+IC0tCj4gCj4gU28gaGVyZSB3ZSBiYXNp Y2FsbHkganVzdCBpZ25vcmUgdGhlIGNvbm5lY3RvciBzdGF0dXMgYW5kIHJldHJhaW4gaXJyZXNw ZWN0aXZlbHkuCgpXZSBpZ25vcmUgdGhlIF9wcmV2aW91c18gY29ubmVjdG9yIHN0YXR1cy4KCj4g QnV0IHRoYXQgbWVhbnMgZXZlbiBpZiB3ZSBoYXZlIG5ld2VyIHZhbHVlcyBub3cgZm9yIG1heCBs aW5rIHJhdGUvbGFuZSBjb3VudCBmcm9tCj4gRFBDRCwgZHVyaW5nIHRoaXMgcmV0cmFpbiB3ZSBh cmUganVzdCB1c2luZyB0aGUgc3RhbGUgdmFsdWUgb2YgaW50ZWxfZHAtPmxpbmtfcmF0ZQo+IGFu ZCBpbnRlbF9kcC0+bGFuZV9jb3VudC4gSSB0aGluayBpbnRlbF9kcC0+bGlua19yYXRlIGFuZCBs YW5lIGNvdW50IHZhbHVlcwo+IHNob3VsZCBiZSBzZXQgdG8gMCBvbiBIUEQgcHVsc2UsIHRoZXkg d291bGQgYmUgc2V0IG9ubHkgZHVyaW5nIGEgbW9kZXNldC4KClRoZSBEUENEIGhhcyBhbHJlYWR5 IGJlZW4gcGFyc2VkIGJ5IHRoaXMgdGltZS4KCi0tIApWaWxsZSBTeXJqw6Rsw6QKSW50ZWwgT1RD Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdm eCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com ([134.134.136.100]:8748 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932193AbdBPRTN (ORCPT ); Thu, 16 Feb 2017 12:19:13 -0500 Date: Thu, 16 Feb 2017 19:18:57 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Manasi Navare Cc: intel-gfx@lists.freedesktop.org, Palmer Dabbelt , stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse Message-ID: <20170216171857.GI31595@intel.com> References: <20170216152659.GD31595@intel.com> <20170216153007.14868-1-ville.syrjala@linux.intel.com> <20170216170753.GA27437@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170216170753.GA27437@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote: > On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� > > > > Apparently some DP sinks are a little nuts and cause HPD to drop > > intermittently during modesets. This happens eg. on an ASUS PB287Q. > > In oder to recover from this we can't really use the previous > > connector status to determine if the link needs retraining, so let's > > just ignore that piece of information and do the retrain > > unconditionally. We do of course still check whether the link is > > supposed to be running or not. > > > > Cc: stable@vger.kernel.org > > Cc: Palmer Dabbelt > > Reported-by: Palmer Dabbelt > > References: https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html > > Signed-off-by: Ville Syrj�l� > > --- > > drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 024798a9c016..37a746f7fbc3 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > > */ > > status = connector_status_disconnected; > > goto out; > > - } else if (connector->status == connector_status_connected) { > > + } else { > > /* > > - * If display was connected already and is still connected > > - * check links status, there has been known issues of > > - * link loss triggerring long pulse!!!! > > + * If display is now connected check links status, > > + * there has been known issues of link loss triggerring > > + * long pulse. > > + * > > + * Some sinks (eg. ASUS PB287Q) seem to perform some > > + * weird HPD ping pong during modesets. So we can apparely > > + * end up with HPD going low during a modeset, and then > > + * going back up soon after. And once that happens we must > > + * retrain the link to get a picture. That's in case no > > + * userspace component reacted to intermittent HPD dip. > > */ > > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > > intel_dp_check_link_status(intel_dp); > > -- > > So here we basically just ignore the connector status and retrain irrespectively. We ignore the _previous_ connector status. > But that means even if we have newer values now for max link rate/lane count from > DPCD, during this retrain we are just using the stale value of intel_dp->link_rate > and intel_dp->lane_count. I think intel_dp->link_rate and lane count values > should be set to 0 on HPD pulse, they would be set only during a modeset. The DPCD has already been parsed by this time. -- Ville Syrj�l� Intel OTC