From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt Date: Mon, 23 Nov 2015 10:43:01 -0500 Message-ID: <1448293381.4081.1.camel@redhat.com> References: <1448034740-30193-1-git-send-email-cpaul@redhat.com> <56507E0F.7030009@amd.com> <56508C34.1030909@amd.com> <1448246674.26668.9.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 514D26E664 for ; Mon, 23 Nov 2015 07:43:04 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: "Deucher, Alexander" , "Koenig, Christian" , Daniel Stone Cc: Jerome Glisse , Benjamin Tissoires , Linux Kernel Mailing List , dri-devel List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAyMDE1LTExLTIzIGF0IDE0OjIwICswMDAwLCBEZXVjaGVyLCBBbGV4YW5kZXIgd3Jv dGU6Cj4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+ID4gRnJvbTogTHl1ZGUgW21haWx0 bzpjcGF1bEByZWRoYXQuY29tXQo+ID4gU2VudDogU3VuZGF5LCBOb3ZlbWJlciAyMiwgMjAxNSA5 OjQ1IFBNCj4gPiBUbzogS29lbmlnLCBDaHJpc3RpYW47IERhbmllbCBTdG9uZQo+ID4gQ2M6IERl dWNoZXIsIEFsZXhhbmRlcjsgRGF2aWQgQWlybGllOyBkcmktZGV2ZWw7IExpbnV4IEtlcm5lbCBN YWlsaW5nIExpc3Q7Cj4gPiBKZXJvbWUgR2xpc3NlOyBCZW5qYW1pbiBUaXNzb2lyZXMKPiA+IFN1 YmplY3Q6IFJlOiBbUEFUQ0hdIGRybS9yYWRlb246IFJldHJ5IEREQyBwcm9iaW5nIG9uIERWSSBv biBmYWlsdXJlIGlmIHdlCj4gPiBnb3QgYW4gSFBEIGludGVycnVwdAo+ID4gCj4gPiBPbiBTYXQs IDIwMTUtMTEtMjEgYXQgMTY6MjIgKzAxMDAsIENocmlzdGlhbiBLw7ZuaWcgd3JvdGU6Cj4gPiA+ IE9uIDIxLjExLjIwMTUgMTU6NDksIERhbmllbCBTdG9uZSB3cm90ZToKPiA+ID4gPiBIaSwKPiA+ ID4gPiAKPiA+ID4gPiBPbiAyMSBOb3ZlbWJlciAyMDE1IGF0IDE0OjIyLCBDaHJpc3RpYW4gS8O2 bmlnIDxjaHJpc3RpYW4ua29lbmlnQGFtZAo+ID4gPiA+IC5jb20+IHdyb3RlOgo+ID4gPiA+ID4g T24gMjAuMTEuMjAxNSAxNjo1MiwgY3BhdWxAcmVkaGF0LmNvbSB3cm90ZToKPiA+ID4gPiA+ID4g VGhpcyBpcyBzb21ld2hhdCByYXJlIG9uIG1vc3QgY2FyZHMgKGRlcGVuZGluZyBvbiB3aGF0IGFu Z2xlCj4gPiA+ID4gPiA+IHlvdSBwbHVnCj4gPiA+ID4gPiA+IHRoZSBEVkkgY29ubmVjdG9yIGlu KSwgYnV0IG9uIHNvbWUgY2FyZHMgaXQgaGFwcGVucyBjb25zdGFudGx5Lgo+ID4gPiA+ID4gPiBU aGUKPiA+ID4gPiA+ID4gUmFkZW9uIFI1IG9uIHRoZSBtYWNoaW5lIHVzZWQgZm9yIHRlc3Rpbmcg dGhpcyBwYXRjaCBmb3IKPiA+ID4gPiA+ID4gaW5zdGFuY2UsIHJ1bnMKPiA+ID4gPiA+ID4gaW50 byB0aGlzIGlzc3VlIGp1c3QgYWJvdXQgZXZlcnkgdGltZSBJIHRyeSB0byBob3RwbHVnIGEgRFZJ Cj4gPiA+ID4gPiA+IG1vbml0b3IgYW5kCj4gPiA+ID4gPiA+IGFzIGEgcmVzdWx0IGhvdHBsdWdn aW5nIGFsbW9zdCBuZXZlciB3b3Jrcy4KPiA+ID4gPiA+ID4gCj4gPiA+ID4gPiA+IFJlc2NoZWR1 bGluZyB0aGUgaG90cGx1ZyB3b3JrIGZvciBhIHNlY29uZCB3aGVuIHdlIHJ1biBpbnRvIGFuCj4g PiA+ID4gPiA+IEhQRAo+ID4gPiA+ID4gPiBzaWduYWwgd2l0aCBhIGZhaWxpbmcgRERDIHByb2Jl IHVzdWFsbHkgZ2l2ZXMgZW5vdWdoIHRpbWUgZm9yCj4gPiA+ID4gPiA+IHRoZSByZXN0Cj4gPiA+ ID4gPiA+IG9mIHRoZSBjb25uZWN0b3IncyBwaW5zIHRvIG1ha2UgY29udGFjdCwgYW5kIGZpeGVz IHRoaXMgaXNzdWUuCj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBTdGVw aGVuIENoYW5kbGVyIFBhdWwgPGNwYXVsQHJlZGhhdC5jb20+Cj4gPiA+ID4gPiAKPiA+ID4gPiA+ IFllYWgsIHRoYXQncyBzb21ldGhpbmcgSSBhbHdheXMgd29uZGVyZWQgYSBhYm91dCBiaXQgYXMg d2VsbC4KPiA+ID4gPiA+IAo+ID4gPiA+ID4gRGVib3VuY2luZyBpcyBzb21ldGhpbmcgdmVyeSBj b21tb24gZG9uZSBpbiBlbGVjdHJvbmljcywgYnV0IGFzCj4gPiA+ID4gPiBmYXIgYXMgSQo+ID4g PiA+ID4ga25vdyB0aGUgSFBEIHBpbnMgZG9uJ3QgbmVjZXNzYXJ5IGhhdmUgYW4gUkMgY2lyY3Vp dCBzbyB3ZSBtaWdodAo+ID4gPiA+ID4gbmVlZCB0bwo+ID4gPiA+ID4gaGFuZGxlIHRoaXMgY2Fz ZSBpbiBzb2Z0d2FyZSBoZXJlLgo+ID4gPiA+ID4gCj4gPiA+ID4gPiBBIGRlbGF5IG9mIHNvbWV0 aGluZyBiZXR3ZWVuIDEwLTMwbXMgYmV0d2VlbiB0aGUgbGFzdCBIUEQKPiA+ID4gPiA+IGludGVy cnVwdCBhbmQKPiA+ID4gPiA+IGZ1cnRoZXIgcHJvY2Vzc2luZyBvZiB0aGUgc2lnbmFsIGRvZXNu J3Qgc291bmRzIGxpa2Ugc3VjaCBhIGJhZAo+ID4gPiA+ID4gaWRlYS4KPiA+IFVuZm9ydHVuYXRl bHkgdGhlIGRlbGF5IG5lZWRlZCB0byBtYWtlIGhvdHBsdWdnaW5nIHdvcmsgb24gdGhlIHN5c3Rl bQo+ID4gbWVudGlvbmVkIGluIHRoZSBjb21taXQgbG9nIGNhbiBhY3R1YWxseSBiZSBvdmVyIDcw MG1zLgo+ID4gPiA+ID4gCj4gPiA+ID4gPiBSZXRyeWluZyBvbiB0aGUgb3RoZXIgaGFuZCBkb2Vz bid0IG5lY2Vzc2FyaWx5IGltcHJvdmUgdGhlCj4gPiA+ID4gPiBzaXR1YXRpb24gY2F1c2UKPiA+ ID4gPiA+IHRoZSBkZWxheSBpbnRyb2R1Y2VkIGJ5IHRoaXMgbWlnaHQgbm90IGJlIGVub3VnaC4K PiA+IFllYWgsIGJ1dCBJIHdvdWxkIHRoaW5rIGl0IHdvdWxkIG1ha2Ugc2Vuc2UgdG8gcmV0cnkg aGVyZSBzbyBsb25nIGFzIHdlCj4gPiBiYWNrIG9mZiBhZnRlciBhIGNlcnRhaW4gdGltZS4gVGhp cyB3b3VsZCBhbHNvIGhhdmUgdGhlIGJlbmVmaXQgb2YKPiA+IHNraXBwaW5nIHRoaXMgZGVsYXkg b24gc3lzdGVtcyB0aGF0IGRvbid0IG5lZWQgaXQuCj4gPiA+ID4gPiAKPiA+ID4gPiA+IFNvIEkg d291bGQgcmF0aGVyIHZvdGUgZm9yIGEgZml4ZWQgZGVsYXkgYmV0d2VlbiBhbiBIUEQgaW50ZXJy dXB0Cj4gPiA+ID4gPiBhbmQKPiA+ID4gPiA+IGFjdHVhbGx5IHN0YXJ0aW5nIHRvIHByb2Nlc3Mg YW55dGhpbmcuCj4gPiA+ID4gWWVzLWlzaC4gRGVib3VuY2luZyBpcyB1c2VmdWwsIGFuZCBpZ25v cmluZyBidWdneSBkZXZpY2VzIChlLmcuCj4gPiA+ID4gdGhvc2UKPiA+ID4gPiBvbiBtYXJnaW5h bCBwb3dlcikgd2hpY2ggc2VuZCB5b3UgSFBEIHN0b3JtcyBhcyB3ZWxsLiBCdXQgRFAgcmVsaWVz Cj4gPiA+ID4gb24KPiA+ID4gPiAnc2hvcnQgSFBEJyBwdWxzZXMgd2hpY2ggY2FuIGJlIGFzIGJy aWVmIGFzIDJtcy4gU28gYXR0ZW1wdGluZyB0bwo+ID4gPiA+IHRvdGFsbHkgZGVib3VuY2UgYWxs IEhQRCB3b24ndCB3b3JrLgo+ID4gPiBXZWxsIHRoZSBkaXNjdXNzaW9uIHNvIGZhciB3YXMgYWJv dXQgSFBEIG9uIERWSSBvbmx5Lgo+ID4gPiAKPiA+ID4gSSdtIG5vdCBzbyBkZWVwIGludG8gRFAs IGJ1dCB3aHkgc2hvdWxkIGl0IHVzZXMgSFBEIHB1bHNlcyBvZiBsZXNzCj4gPiA+IHRoYW4gMm1z Pwo+ID4gVGhpcyBpcyBwYXJ0IG9mIHRoZSBEUCBzcGVjIGlpcmMuIFRoaXMgYmVpbmcgc2FpZCB0 aG91Z2gsIHRoZSBpc3N1ZQo+ID4gaGVyZSB3aXRoIHRoZSBIUEQgc2lnbmFsIGNvbWluZyBiZWZv cmUgdGhlIGNvbm5lY3RvciBpcyByZWFkeSBvbmx5Cj4gPiBoYXBwZW5zIG9uIERWSS4gSSBoYXZl bid0IGV2ZXIgcnVuIGludG8gdGhpcyBpc3N1ZSB3aXRoIGFueSBIRE1JIGNhYmxlcwo+ID4gb3Ig RFAgY2FibGVzLCBzbyBJJ20gYWdhaW5zdCBpbXBvc2luZyB0aGlzIG9uIGFsbCBjb25uZWN0b3Jz Lgo+ID4gCj4gPiBPbmUgb2YgdGhlIHNvbHV0aW9ucyBJJ3ZlIGJlZW4gdGhpbmtpbmcgYWJvdXQg d2l0aCB0aGlzOiBJbgo+ID4gcmFkZW9uX2R2aV9kZXRlY3QoKSwgaWYgd2UgZ2V0IGEgcmVhbCBo b3RwbHVnIHNpZ25hbCByZXRyeSB0aGUgRERDCj4gPiBwcm9iZSB1bnRpbCBhdCBsZWFzdCBhIHNl Y29uZCBoYXMgcGFzc2VkLCBhZnRlciB3aGljaCB3ZSBiYWNrIG9mZiBhbmQKPiA+IGFzc3VtZSB0 aGUgcG9ydCBpcyBkaXNjb25uZWN0ZWQuCj4gCj4gRldJVywgdGhlcmUgYXJlIHJlZ2lzdGVycyB0 byBhZGp1c3QgaG93IGxvbmcgdGhlIGhwZCBuZWVkcyB0byBiZSBhc3NlcnRlZAo+IGJlZm9yZSB0 aGUgaHBkIGNvbm5lY3Rpb24gYW5kIHNob3J0IHB1bHNlIGludGVycnVwdHMgYXJlIHRyaWdnZXJl ZC7CoMKgU2VlCj4gRENfSFBEeF9DT05UUk9MLsKgwqBNYXliZSBhZGp1c3RpbmcgdGhlbSB3b3Vs ZCBoZWxwLsKgwqBXZSBjdXJyZW50bHkganVzdCB3cml0ZQo+IHRoZSBkZWZhdWx0IHZhbHVlLCBi dXQgaXQgbWlnaHQgYmUgYmV0dGVyIHRvIFJNVyB0aGUgdmFsdWUgaW4gY2FzZSB0aGVyZSBpcyBh Cj4gc3BlY2lhbCBnb2xkZW4gdmFsdWUgc2V0IGJ5IHRoZSB2YmlvcyBhdCBpbml0IHRpbWUuCj4g ClRoaXMgc291bmRzIGxpa2UgYSBtdWNoIGJldHRlciBpZGVhLiBJJ20gZ29pbmcgdG8gdHJ5IHRv IGNvbWUgdXAgd2l0aCBhIHBhdGNoCnRoYXQgYWRqdXN0cyB0aGVzZSBieSBkZWZhdWx0IGZvciBE VkkgcG9ydHMuCj4gQWxleAo+IAo+ID4gPiAKPiA+ID4gUmVnYXJkcywKPiA+ID4gQ2hyaXN0aWFu Lgo+ID4gPiAKPiA+ID4gPiAKPiA+ID4gPiBDaGVlcnMsCj4gPiA+ID4gRGFuaWVsCj4gPiA+IAo+ ID4gLS0KPiA+IENoZWVycywKPiA+IAlMeXVkZQo+IAotLSAKQ2hlZXJzLAoJTHl1ZGUKCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753598AbbKWPnG (ORCPT ); Mon, 23 Nov 2015 10:43:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44444 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753266AbbKWPnE (ORCPT ); Mon, 23 Nov 2015 10:43:04 -0500 Message-ID: <1448293381.4081.1.camel@redhat.com> Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt From: Lyude To: "Deucher, Alexander" , "Koenig, Christian" , Daniel Stone Cc: David Airlie , dri-devel , Linux Kernel Mailing List , Jerome Glisse , Benjamin Tissoires Date: Mon, 23 Nov 2015 10:43:01 -0500 In-Reply-To: References: <1448034740-30193-1-git-send-email-cpaul@redhat.com> <56507E0F.7030009@amd.com> <56508C34.1030909@amd.com> <1448246674.26668.9.camel@redhat.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote: > > -----Original Message----- > > From: Lyude [mailto:cpaul@redhat.com] > > Sent: Sunday, November 22, 2015 9:45 PM > > To: Koenig, Christian; Daniel Stone > > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List; > > Jerome Glisse; Benjamin Tissoires > > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we > > got an HPD interrupt > > > > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote: > > > On 21.11.2015 15:49, Daniel Stone wrote: > > > > Hi, > > > > > > > > On 21 November 2015 at 14:22, Christian König > > > .com> wrote: > > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote: > > > > > > This is somewhat rare on most cards (depending on what angle > > > > > > you plug > > > > > > the DVI connector in), but on some cards it happens constantly. > > > > > > The > > > > > > Radeon R5 on the machine used for testing this patch for > > > > > > instance, runs > > > > > > into this issue just about every time I try to hotplug a DVI > > > > > > monitor and > > > > > > as a result hotplugging almost never works. > > > > > > > > > > > > Rescheduling the hotplug work for a second when we run into an > > > > > > HPD > > > > > > signal with a failing DDC probe usually gives enough time for > > > > > > the rest > > > > > > of the connector's pins to make contact, and fixes this issue. > > > > > > > > > > > > Signed-off-by: Stephen Chandler Paul > > > > > > > > > > Yeah, that's something I always wondered a about bit as well. > > > > > > > > > > Debouncing is something very common done in electronics, but as > > > > > far as I > > > > > know the HPD pins don't necessary have an RC circuit so we might > > > > > need to > > > > > handle this case in software here. > > > > > > > > > > A delay of something between 10-30ms between the last HPD > > > > > interrupt and > > > > > further processing of the signal doesn't sounds like such a bad > > > > > idea. > > Unfortunately the delay needed to make hotplugging work on the system > > mentioned in the commit log can actually be over 700ms. > > > > > > > > > > Retrying on the other hand doesn't necessarily improve the > > > > > situation cause > > > > > the delay introduced by this might not be enough. > > Yeah, but I would think it would make sense to retry here so long as we > > back off after a certain time. This would also have the benefit of > > skipping this delay on systems that don't need it. > > > > > > > > > > So I would rather vote for a fixed delay between an HPD interrupt > > > > > and > > > > > actually starting to process anything. > > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. > > > > those > > > > on marginal power) which send you HPD storms as well. But DP relies > > > > on > > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to > > > > totally debounce all HPD won't work. > > > Well the discussion so far was about HPD on DVI only. > > > > > > I'm not so deep into DP, but why should it uses HPD pulses of less > > > than 2ms? > > This is part of the DP spec iirc. This being said though, the issue > > here with the HPD signal coming before the connector is ready only > > happens on DVI. I haven't ever run into this issue with any HDMI cables > > or DP cables, so I'm against imposing this on all connectors. > > > > One of the solutions I've been thinking about with this: In > > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC > > probe until at least a second has passed, after which we back off and > > assume the port is disconnected. > > FWIW, there are registers to adjust how long the hpd needs to be asserted > before the hpd connection and short pulse interrupts are triggered.  See > DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write > the default value, but it might be better to RMW the value in case there is a > special golden value set by the vbios at init time. > This sounds like a much better idea. I'm going to try to come up with a patch that adjusts these by default for DVI ports. > Alex > > > > > > > Regards, > > > Christian. > > > > > > > > > > > Cheers, > > > > Daniel > > > > > -- > > Cheers, > > Lyude > -- Cheers, Lyude