From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sharma, Shashank" Subject: Re: [PATCH 2/4] drm/i915/lspcon: Fix resume time initialization due to unasserted HPD Date: Sun, 29 Jan 2017 10:33:07 +0530 Message-ID: <46bb02bb-a555-e2f7-dc80-0d0cdfd8cd45@intel.com> References: <1485509961-9010-1-git-send-email-imre.deak@intel.com> <1485509961-9010-3-git-send-email-imre.deak@intel.com> <20170128081742.GB32065@ideak-desk.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id E28F76E0D5 for ; Sun, 29 Jan 2017 05:03:11 +0000 (UTC) In-Reply-To: <20170128081742.GB32065@ideak-desk.fi.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: imre.deak@intel.com Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org UmVnYXJkcwoKU2hhc2hhbmsKCgpPbiAxLzI4LzIwMTcgMTo0NyBQTSwgSW1yZSBEZWFrIHdyb3Rl Ogo+IE9uIFNhdCwgSmFuIDI4LCAyMDE3IGF0IDEwOjMyOjAzQU0gKzA1MzAsIFNoYXJtYSwgU2hh c2hhbmsgd3JvdGU6Cj4+IFJlZ2FyZHMKPj4KPj4gU2hhc2hhbmsKPj4KPj4KPj4gT24gMS8yNy8y MDE3IDM6MDkgUE0sIEltcmUgRGVhayB3cm90ZToKPj4+IER1cmluZyBzeXN0ZW0gcmVzdW1lIHRp bWUgaW5pdGlhbGl6YXRpb24gdGhlIEhQRCBsZXZlbCBvbiBMU1BDT04gcG9ydHMKPj4+IGNhbiBz dGF5IGxvdyBmb3IgYW4gZXh0ZW5kZWQgYW1vdW50IG9mIHRpbWUsIGxlYWRpbmcgdG8gZmFpbGVk IEFVWAo+Pj4gdHJhbnNmZXJzIGFuZCBMU1BDT04gaW5pdGlhbGl6YXRpb24uIEZpeCB0aGlzIGJ5 IHdhaXRpbmcgZm9yIEhQRCB0byBnZXQKPj4+IGFzc2VydGVkLgo+Pj4KPj4+IEJ1Z3ppbGxhOiBo dHRwczovL2J1Z3MuZnJlZWRlc2t0b3Aub3JnL3Nob3dfYnVnLmNnaT9pZD05OTE3OAo+Pj4gQ2M6 IFNoYXNoYW5rIFNoYXJtYSA8c2hhc2hhbmsuc2hhcm1hQGludGVsLmNvbT4KPj4+IENjOiBKYW5p IE5pa3VsYSA8amFuaS5uaWt1bGFAbGludXguaW50ZWwuY29tPgo+Pj4gQ2M6IFZpbGxlIFN5cmrD pGzDpCA8dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4+PiBDYzogRGFuaWVsIFZldHRl ciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPj4+IENjOiA8c3RhYmxlQHZnZXIua2VybmVsLm9y Zz4gIyB2NC45Kwo+Pj4gU2lnbmVkLW9mZi1ieTogSW1yZSBEZWFrIDxpbXJlLmRlYWtAaW50ZWwu Y29tPgo+Pj4gLS0tCj4+PiAgIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMgICAgIHwg NCArKy0tCj4+PiAgIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5oICAgIHwgMiArKwo+ Pj4gICBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9sc3Bjb24uYyB8IDUgKysrKy0KPj4+ICAg MyBmaWxlcyBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCj4+Pgo+Pj4g ZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMgYi9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pbnRlbF9kcC5jCj4+PiBpbmRleCBlMGY5YjllLi5hNzc4NWNlIDEwMDY0NAo+ Pj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+Pj4gKysrIGIvZHJpdmVy cy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+Pj4gQEAgLTQ0MDAsOCArNDQwMCw4IEBAIHN0YXRp YyBib29sIGJ4dF9kaWdpdGFsX3BvcnRfY29ubmVjdGVkKHN0cnVjdCBkcm1faTkxNV9wcml2YXRl ICpkZXZfcHJpdiwKPj4+ICAgICoKPj4+ICAgICogUmV0dXJuICV0cnVlIGlmIEBwb3J0IGlzIGNv bm5lY3RlZCwgJWZhbHNlIG90aGVyd2lzZS4KPj4+ICAgICovCj4+PiAtc3RhdGljIGJvb2wgaW50 ZWxfZGlnaXRhbF9wb3J0X2Nvbm5lY3RlZChzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3By aXYsCj4+PiAtCQkJCQkgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCAqcG9ydCkKPj4+ICtib29s IGludGVsX2RpZ2l0YWxfcG9ydF9jb25uZWN0ZWQoc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRl dl9wcml2LAo+Pj4gKwkJCQkgIHN0cnVjdCBpbnRlbF9kaWdpdGFsX3BvcnQgKnBvcnQpCj4+PiAg IHsKPj4+ICAgCWlmIChIQVNfUENIX0lCWChkZXZfcHJpdikpCj4+PiAgIAkJcmV0dXJuIGlieF9k aWdpdGFsX3BvcnRfY29ubmVjdGVkKGRldl9wcml2LCBwb3J0KTsKPj4+IGRpZmYgLS1naXQgYS9k cml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcnYuaCBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2lu dGVsX2Rydi5oCj4+PiBpbmRleCBiNzFmZWNlLi5iOWNkZTExIDEwMDY0NAo+Pj4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmgKPj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX2Rydi5oCj4+PiBAQCAtMTQ4OSw2ICsxNDg5LDggQEAgYm9vbCBfX2ludGVsX2Rw X3JlYWRfZGVzYyhzdHJ1Y3QgaW50ZWxfZHAgKmludGVsX2RwLAo+Pj4gICBib29sIGludGVsX2Rw X3JlYWRfZGVzYyhzdHJ1Y3QgaW50ZWxfZHAgKmludGVsX2RwKTsKPj4+ICAgaW50IGludGVsX2Rw X2xpbmtfcmVxdWlyZWQoaW50IHBpeGVsX2Nsb2NrLCBpbnQgYnBwKTsKPj4+ICAgaW50IGludGVs X2RwX21heF9kYXRhX3JhdGUoaW50IG1heF9saW5rX2Nsb2NrLCBpbnQgbWF4X2xhbmVzKTsKPj4+ ICtib29sIGludGVsX2RpZ2l0YWxfcG9ydF9jb25uZWN0ZWQoc3RydWN0IGRybV9pOTE1X3ByaXZh dGUgKmRldl9wcml2LAo+Pj4gKwkJCQkgIHN0cnVjdCBpbnRlbF9kaWdpdGFsX3BvcnQgKnBvcnQp Owo+Pj4gICAvKiBpbnRlbF9kcF9hdXhfYmFja2xpZ2h0LmMgKi8KPj4+ICAgaW50IGludGVsX2Rw X2F1eF9pbml0X2JhY2tsaWdodF9mdW5jcyhzdHJ1Y3QgaW50ZWxfY29ubmVjdG9yICppbnRlbF9j b25uZWN0b3IpOwo+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2xz cGNvbi5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfbHNwY29uLmMKPj4+IGluZGV4IGY2 ZDRlNjkuLmMzMDA2NDcgMTAwNjQ0Cj4+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRl bF9sc3Bjb24uYwo+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfbHNwY29uLmMK Pj4+IEBAIC0xNTgsNiArMTU4LDggQEAgc3RhdGljIGJvb2wgbHNwY29uX3Byb2JlKHN0cnVjdCBp bnRlbF9sc3Bjb24gKmxzcGNvbikKPj4+ICAgc3RhdGljIHZvaWQgbHNwY29uX3Jlc3VtZV9pbl9w Y29uX3dhKHN0cnVjdCBpbnRlbF9sc3Bjb24gKmxzcGNvbikKPj4+ICAgewo+Pj4gICAJc3RydWN0 IGludGVsX2RwICppbnRlbF9kcCA9IGxzcGNvbl90b19pbnRlbF9kcChsc3Bjb24pOwo+Pj4gKwlz dHJ1Y3QgaW50ZWxfZGlnaXRhbF9wb3J0ICpkaWdfcG9ydCA9IGRwX3RvX2RpZ19wb3J0KGludGVs X2RwKTsKPj4+ICsJc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2ID0gdG9faTkxNShk aWdfcG9ydC0+YmFzZS5iYXNlLmRldik7Cj4+PiAgIAl1bnNpZ25lZCBsb25nIHN0YXJ0ID0gamlm ZmllczsKPj4+ICAgCWlmICghbHNwY29uLT5kZXNjX3ZhbGlkKQo+Pj4gQEAgLTE3Myw3ICsxNzUs OCBAQCBzdGF0aWMgdm9pZCBsc3Bjb25fcmVzdW1lX2luX3Bjb25fd2Eoc3RydWN0IGludGVsX2xz cGNvbiAqbHNwY29uKQo+Pj4gICAJCWlmICghX19pbnRlbF9kcF9yZWFkX2Rlc2MoaW50ZWxfZHAs ICZkZXNjKSkKPj4+ICAgCQkJcmV0dXJuOwo+Pj4gLQkJaWYgKCFtZW1jbXAoJmludGVsX2RwLT5k ZXNjLCAmZGVzYywgc2l6ZW9mKGRlc2MpKSkgewo+Pj4gKwkJaWYgKGludGVsX2RpZ2l0YWxfcG9y dF9jb25uZWN0ZWQoZGV2X3ByaXYsIGRpZ19wb3J0KSAmJgo+PiB3aGVuIGluIFBDT04gbW9kZSwg bGl2ZV9zdGF0dXMgaXMgYWx3YXlzIHVwIGZvciBMU1BDT04gcG9ydCwgc28gdGhpcyBjaGVjawo+ PiB3aWxsIGJlIGFsd2F5cyB0cnVlLCBpc24ndCBpdCA/Cj4gTm90IGF0IGxlYXN0IGluIGNhc2Ug b2YgdGhlIE1lZ2FDaGlwcyBMU1BDT04gSSd2ZSBzZWVuLCB0aGVyZSBpdCB0YWtlcyBhCj4gd2hp bGUgZm9yIEhQRCB0byBnZXQgYXNzZXJ0ZWQuIEFuZCB3aGlsZSBpdCdzIG5vdCBhc3NlcnRlZCBB VVgKPiB0cmFuc2FjdGlvbnMgd2lsbCBlaXRoZXI6Cj4gLSByZXR1cm4gZ2FyYmFnZSBpbiBjYXNl IG9mIG5hdGl2ZSBBVVggdHJhbnNhY3Rpb25zCj4gLSBoYW5nIHRoZSBjaGlwL0ZXIHVudGlsIG5l eHQgY29sZCByZWJvb3QgaW4gY2FzZSBvZiBJMkMtb3Zlci1BVVgKPiAgICB0cmFuc2FjdGlvbnMK PiAtIHNpbXBseSBub3QgZ2V0IGEgcmVwbHkgaWYgdGhlIGNoaXAvRlcgaW5pdGlhbGl6YXRpb24g aGFzIHJlYWNoZWQgYQo+ICAgIGNlcnRhaW4gcGhhc2UKPgo+IEJhc2VkIG9uIHRoZSBEUCBzcGVj aWZpY2F0aW9uIHRoZSBzaW5rL2JyYW5jaCBkZXZpY2UgaXMgbm90IHJlcXVpcmVkIHRvCj4gcmVz cG9uZCBpbiBjYXNlIHRoZSBIUEQgaXMgbm90IGFzc2VydGVkLCBzbyB0aGUgM3JkIHNjZW5hcmlv IGlzCj4gY29tcGxpYW50LCBidXQgdGhlIGZpcnN0IHR3byBhcmUgbm90Lgo+Cj4gSW4gUENPTiBt b2RlIGFmdGVyIGluaXRpYWxpemF0aW9uIGFuZCBIUEQgZ2V0dGluZyBhc3NlcnRlZCwgSFBEIHdp bGwKPiBzdGF5IGFzc2VydGVkIGV4Y2VwdCBmb3IgdGhlIHNob3J0IHB1bHNlcyBzaWduYWxpbmcg YW4gb3V0cHV0Cj4gY29ubmVjdC9kaXNjb25uZWN0LgpUaGUgcmVhc29uIG1pZ2h0IGJlLCB0aGF0 IExTUENPTiByZXN1bWVzIGluIExTIHR5cGUyIGFkYXB0ZXIgc3RhdGUsIAp3aGVyZSB0aGUgbGl2 ZV9zdGF0dXMgYmVoYXZpb3IgaXMgbm9ybWFsCmFuZCByZWZsZWN0cyB0aGUgcmVhbCBIUEQgbGlu ZSwgYnV0IHRoZSBtb21lbnQgd2UgbW92ZSB0byBQQ09OIG1vZGUsIEhQRCAKZ2V0cyBhc3NlcnRl ZCBsb3cuCkkga25vdyB5b3Ugd291bGQgaGF2ZSB0ZXN0ZWQgdGhpcyB3ZWxsLCBidXQgSSBhbHNv IHdhbnQgdG8gdGVzdCB0aGlzIApjb2RlIGFuZCBsb2dpYyBmaXJzdCwgYmVmb3JlIHdlIGdvIGFo ZWFkIHdpdGggdGhlIHBhdGNoLgoKU2hhc2hhbmsKPiAtLUltcmUKPgo+PiAtIFNoYXNoYW5rCj4+ PiArCQkgICAgIW1lbWNtcCgmaW50ZWxfZHAtPmRlc2MsICZkZXNjLCBzaXplb2YoZGVzYykpKSB7 Cj4+PiAgIAkJCURSTV9ERUJVR19LTVMoIkxTUENPTiByZWNvdmVyaW5nIGluIFBDT04gbW9kZSBh ZnRlciAldSBtc1xuIiwKPj4+ICAgCQkJCSAgICAgIGppZmZpZXNfdG9fbXNlY3MoamlmZmllcyAt IHN0YXJ0KSk7Cj4+PiAgIAkJCXJldHVybjsKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3Rz LmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com ([134.134.136.31]:16833 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbdA2FQB (ORCPT ); Sun, 29 Jan 2017 00:16:01 -0500 Subject: Re: [PATCH 2/4] drm/i915/lspcon: Fix resume time initialization due to unasserted HPD To: imre.deak@intel.com References: <1485509961-9010-1-git-send-email-imre.deak@intel.com> <1485509961-9010-3-git-send-email-imre.deak@intel.com> <20170128081742.GB32065@ideak-desk.fi.intel.com> Cc: intel-gfx@lists.freedesktop.org, Jani Nikula , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Daniel Vetter , stable@vger.kernel.org From: "Sharma, Shashank" Message-ID: <46bb02bb-a555-e2f7-dc80-0d0cdfd8cd45@intel.com> Date: Sun, 29 Jan 2017 10:33:07 +0530 MIME-Version: 1.0 In-Reply-To: <20170128081742.GB32065@ideak-desk.fi.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: Regards Shashank On 1/28/2017 1:47 PM, Imre Deak wrote: > On Sat, Jan 28, 2017 at 10:32:03AM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 1/27/2017 3:09 PM, Imre Deak wrote: >>> During system resume time initialization the HPD level on LSPCON ports >>> can stay low for an extended amount of time, leading to failed AUX >>> transfers and LSPCON initialization. Fix this by waiting for HPD to get >>> asserted. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99178 >>> Cc: Shashank Sharma >>> Cc: Jani Nikula >>> Cc: Ville Syrj�l� >>> Cc: Daniel Vetter >>> Cc: # v4.9+ >>> Signed-off-by: Imre Deak >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 4 ++-- >>> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >>> drivers/gpu/drm/i915/intel_lspcon.c | 5 ++++- >>> 3 files changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>> index e0f9b9e..a7785ce 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -4400,8 +4400,8 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv, >>> * >>> * Return %true if @port is connected, %false otherwise. >>> */ >>> -static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> - struct intel_digital_port *port) >>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> + struct intel_digital_port *port) >>> { >>> if (HAS_PCH_IBX(dev_priv)) >>> return ibx_digital_port_connected(dev_priv, port); >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index b71fece..b9cde11 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -1489,6 +1489,8 @@ bool __intel_dp_read_desc(struct intel_dp *intel_dp, >>> bool intel_dp_read_desc(struct intel_dp *intel_dp); >>> int intel_dp_link_required(int pixel_clock, int bpp); >>> int intel_dp_max_data_rate(int max_link_clock, int max_lanes); >>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv, >>> + struct intel_digital_port *port); >>> /* intel_dp_aux_backlight.c */ >>> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector); >>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c >>> index f6d4e69..c300647 100644 >>> --- a/drivers/gpu/drm/i915/intel_lspcon.c >>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c >>> @@ -158,6 +158,8 @@ static bool lspcon_probe(struct intel_lspcon *lspcon) >>> static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon) >>> { >>> struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon); >>> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >>> + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); >>> unsigned long start = jiffies; >>> if (!lspcon->desc_valid) >>> @@ -173,7 +175,8 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon) >>> if (!__intel_dp_read_desc(intel_dp, &desc)) >>> return; >>> - if (!memcmp(&intel_dp->desc, &desc, sizeof(desc))) { >>> + if (intel_digital_port_connected(dev_priv, dig_port) && >> when in PCON mode, live_status is always up for LSPCON port, so this check >> will be always true, isn't it ? > Not at least in case of the MegaChips LSPCON I've seen, there it takes a > while for HPD to get asserted. And while it's not asserted AUX > transactions will either: > - return garbage in case of native AUX transactions > - hang the chip/FW until next cold reboot in case of I2C-over-AUX > transactions > - simply not get a reply if the chip/FW initialization has reached a > certain phase > > Based on the DP specification the sink/branch device is not required to > respond in case the HPD is not asserted, so the 3rd scenario is > compliant, but the first two are not. > > In PCON mode after initialization and HPD getting asserted, HPD will > stay asserted except for the short pulses signaling an output > connect/disconnect. The reason might be, that LSPCON resumes in LS type2 adapter state, where the live_status behavior is normal and reflects the real HPD line, but the moment we move to PCON mode, HPD gets asserted low. I know you would have tested this well, but I also want to test this code and logic first, before we go ahead with the patch. Shashank > --Imre > >> - Shashank >>> + !memcmp(&intel_dp->desc, &desc, sizeof(desc))) { >>> DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n", >>> jiffies_to_msecs(jiffies - start)); >>> return;