From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Refresh cached DP port register value on resume Date: Wed, 22 Jun 2016 21:13:05 +0300 Message-ID: <20160622181305.GE4329@intel.com> References: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> <1466188845.2572.16.camel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id CD7B96E825 for ; Wed, 22 Jun 2016 18:13:09 +0000 (UTC) Content-Disposition: inline In-Reply-To: <1466188845.2572.16.camel@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 Cc: Jani Nikula , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gRnJpLCBKdW4gMTcsIDIwMTYgYXQgMDk6NDA6NDVQTSArMDMwMCwgSW1yZSBEZWFrIHdyb3Rl Ogo+IE9uIEZyaSwgMjAxNi0wNS0xMyBhdCAyMDo1MyArMDMwMCwgdmlsbGUuc3lyamFsYUBsaW51 eC5pbnRlbC5jb20gd3JvdGU6Cj4gPiBGcm9tOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmph bGFAbGludXguaW50ZWwuY29tPgo+ID4gCj4gPiBEdXJpbmcgaGliZXJuYXRpb24gdGhlIGNhY2hl ZCBEUCBwb3J0IHJlZ2lzdGVyIHZhbHVlIHdpbGwgYmUgbGVmdCB3aXRoCj4gPiB3aGF0ZXZlciB2 YWx1ZSB3ZSBoYXZlIHRoZXJlIHdoZW4gd2UgY3JlYXRlIHRoZSBoaWJlcm5hdGlvbiBpbWFnZS4K PiA+IEN1cnJlbnRseSB0aGF0IG1lYW5zIHRoZSBwb3J0IChhbmQgZURQIFBMTCkgd2lsbCBiZSBv ZmYgaW4gdGhlIGNhY2hlZAo+ID4gdmFsdWUuIEhvd2V2ZXIgd2hlbiB3ZSByZXN1bWUgdGhlcmUg aXMgbm8gZ3VhcmFudGVlIHRoYXQgdGhlIHZhbHVlCj4gPiBpbiB0aGUgYWN0dWFsIHJlZ2lzdGVy IHdpbGwgbWF0Y2ggdGhlIGNhY2hlZCB2YWx1ZS4gSWYgaTkxNSBpc24ndAo+ID4gbG9hZGVkIGlu IHRoZSBrZXJuZWwgdGhhdCBsb2FkcyB0aGUgaGliZXJuYXRpb24gaW1hZ2UsIHRoZSBwb3J0IG1h eQo+ID4gd2VsbCBiZSBvbiAoZWcuIGxlZnQgb24gYnkgdGhlIEJJT1MpLiBUaGUgZW5jb2RlciBz dGF0ZSByZWFkb3V0Cj4gPiBkb2VzIHRoZSByaWdodCB0aGluZyBpbiB0aGlzIGNhc2UgYW5kIHVw ZGF0ZXMgb3VyIGVuY29kZXIgc3RhdGUKPiA+IHRvIHJlZmxlY3QgdGhlIGFjdHVhbCBoYXJkd2Fy ZSBzdGF0ZS4gSG93ZXZlciB0aGUgcG9zdC1yZXN1bWUgbW9kZXNldAo+ID4gd2lsbCB0aGVuIHVz ZSB0aGUgc3RhbGUgY2FjaGVkIHBvcnQgcmVnaXN0ZXIgdmFsdWUgaW4KPiA+IGludGVsX2RwX2xp bmtfZG93bigpIGFuZCBwb3RlbnRpYWxseSBjb25mdXNlIHRoZSBoYXJkd2FyZS4KPiA+IAo+ID4g VGhpcyB3YXMgY2F1Z2h0IGJ5IHRoZSBmb2xsb3dpbmcgYXNzZXJ0Cj4gPiDCoFdBUk5JTkc6IENQ VTogMyBQSUQ6IDUyODggYXQgLi4vZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYzoyMTg0 IGFzc2VydF9lZHBfcGxsKzB4OTkvMHhhMCBbaTkxNV0KPiA+IMKgZURQIFBMTCBzdGF0ZSBhc3Nl cnRpb24gZmFpbHVyZSAoZXhwZWN0ZWQgb24sIGN1cnJlbnQgb2ZmKQo+ID4gb24gYWNjb3VudCBv ZiB0aGUgZURQIFBMTCBnZXR0aW5nIHByZW1hdHVyZWx5IHR1cm5lZCBvZmYgd2hlbgo+ID4gc2h1 dHRpbmcgZG93biB0aGUgcG9ydCwgc2luY2UgdGhlIERQX1BMTF9FTkFCTEUgYml0IHdhc24ndCBz ZXQKPiA+IGluIHRoZSBjYWNoZWQgcmVnaXN0ZXIgdmFsdWUuCj4gPiAKPiA+IFByZXN1bWFibHkg SSBpbnRyb2R1Y2VkIHRoaXMgcHJvYmxlbSBpbgo+ID4gY29tbWl0IDZmZWM3NjYyODMzMyAoImRy bS9pOTE1OiBVc2UgaW50ZWxfZHAtPkRQIGluIGVEUCBQTEwgc2V0dXAiKQo+ID4gYXMgYmVmb3Jl IHRoYXQgd2UgZGlkbid0IHVwZGF0ZSB0aGUgY2FjaGVkIHZhbHVlIGFmdGVyIHNodXR0dGluZyB0 aGUKPiA+IHBvcnQgZG93bi4gVGhhdCdzIGFzc3VtaW5nIHRoZSBwb3J0IGdvdCBlbmFibGVkIGF0 IGxlYXN0IG9uY2UgcHJpb3IKPiA+IHRvIGhpYmVybmF0aW5nLiBJZiB0aGF0IGRpZG4ndCBoYXBw ZW4gdGhlbiB0aGUgY2FjaGVkIHZhbHVlIHdvdWxkCj4gPiBzdGlsbCBoYXZlIGJlZW4gdG90YWxs eSBvdXQgb2Ygc3luYyB3aXRoIHJlYWxpdHkgKGVnLiBmaXJzdCBib290IHcvbwo+ID4gZURQIG9u LCB0aGVuIGhpYmVybmF0ZSwgYW5kIHRoZW4gcmVzdW1lIHdpdGggZURQIG9uKS4KPiA+IAo+ID4g U28sIGxldCdzIGZpeCB0aGlzIHByb3Blcmx5IGFuZCByZWZyZXNoIHRoZSBjYWNoZWQgcmVnaXN0 ZXIgdmFsdWUgZnJvbQo+ID4gdGhlIGhhcmR3YXJlIHJlZ2lzdGVyIGR1cmluZyByZXN1bWUuCj4g PiAKPiA+IERESSBwbGF0Zm9ybXMgc2hvdWxkbid0IHVzZSB0aGUgY2FjaGVkIHZhbHVlIGR1cmlu ZyBwb3J0IGRpc2FibGUgYXQKPiA+IGxlYXN0LCBzbyBzaG91bGRuJ3QgaGF2ZSB0aGlzIHBhcnRp Y3VsYXIgaXNzdWUuIFRoZXkgbWlnaHQgc3RpbGwgaGF2ZQo+ID4gaXNzdWVzIGlmIHdlIHNraXAg dGhlIGluaXRpYWwgbW9kZXNldCBhbmQgdGhlbiB0cnkgdG8gcmV0cmFpbiB0aGUgbGluawo+ID4g b3Igc29tZXRoaW5nLiBCdXQgdW50YW5nbGluZyB0aGlzIERQIHZzLiBEREkgbWVzcyBpcyBhIGJp Z2dlciB0b3BpYywKPiA+IHNvIGxldCdzIGp1dCBwdW50IG9uIERESSBmb3Igbm93Lgo+IAo+IFNp bmNlIHRoZSBEREkgbGluayByZXRyYWluaW5nIGNvZGUgc2VlbXMgdG8gcmVzZXQgYWxsIHJlbGV2 YW50IHBhcnRzIG9mCj4gaW50ZWxfZHAtPkRQIChsYW5lLCB2c3dpbmcsIHBvcnQgZW5hYmxlZCkg d291bGQgdGhlIGFib3ZlIHNjZW5hcmlvIGJlCj4gcmVhbGx5IGEgcHJvYmxlbT8gQnV0IHN5bmNp bmcgaW50ZWxfZHAtPkRQIHRoZSBzYW1lIHdheSBmb3IgRERJIG1ha2VzCj4gc2Vuc2UgdG8gbWUg aW4gYW55IGNhc2UuCgpJIGRpZG4ndCBsb29rIHRvbyBjbG9zZWx5IGF0IHRoZSBEREkgY2FzZSBz byBmYXIsIHNvIG5vdCBzdXJlIHdoYXQncwpnb2luZyBvbiB0aGVyZS4KCj4gCj4gPiBDYzogSmFu aSBOaWt1bGEgPGphbmkubmlrdWxhQGludGVsLmNvbT4KPiA+IENjOiBzdGFibGVAdmdlci5rZXJu ZWwub3JnCj4gPiBGaXhlczogNmZlYzc2NjI4MzMzICgiZHJtL2k5MTU6IFVzZSBpbnRlbF9kcC0+ RFAgaW4gZURQIFBMTCBzZXR1cCIpCj4gPiBTaWduZWQtb2ZmLWJ5OiBWaWxsZSBTeXJqw6Rsw6Qg PHZpbGxlLnN5cmphbGFAbGludXguaW50ZWwuY29tPgo+IAo+IFJldmlld2VkLWJ5OiBJbXJlIERl YWsgPGltcmUuZGVha0BpbnRlbC5jb20+CgpQdXNoZWQgdG8gZGlucS4gVGhhbmtzIGZvciB0aGUg cmV2aWV3LgoKPiAKPiA+IC0tLQo+ID4gwqBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcC5j IHwgOCArKysrKy0tLQo+ID4gwqAxIGZpbGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCAzIGRl bGV0aW9ucygtKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50 ZWxfZHAuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMKPiA+IGluZGV4IDM2MzMw MDI2Y2VmZi4uYTNmMzgxMTVhM2JkIDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5 MTUvaW50ZWxfZHAuYwo+ID4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+ ID4gQEAgLTQ1MjIsMTMgKzQ1MjIsMTUgQEAgc3RhdGljIHZvaWQgaW50ZWxfZWRwX3BhbmVsX3Zk ZF9zYW5pdGl6ZShzdHJ1Y3QgaW50ZWxfZHAgKmludGVsX2RwKQo+ID4gwqAKPiA+IMKgdm9pZCBp bnRlbF9kcF9lbmNvZGVyX3Jlc2V0KHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlcikKPiA+IMKg ewo+ID4gLQlzdHJ1Y3QgaW50ZWxfZHAgKmludGVsX2RwOwo+ID4gKwlzdHJ1Y3QgZHJtX2k5MTVf cHJpdmF0ZSAqZGV2X3ByaXYgPSB0b19pOTE1KGVuY29kZXItPmRldik7Cj4gPiArCXN0cnVjdCBp bnRlbF9kcCAqaW50ZWxfZHAgPSBlbmNfdG9faW50ZWxfZHAoZW5jb2Rlcik7Cj4gPiArCj4gPiAr CWlmICghSEFTX0RESShkZXZfcHJpdikpCj4gPiArCQlpbnRlbF9kcC0+RFAgPSBJOTE1X1JFQUQo aW50ZWxfZHAtPm91dHB1dF9yZWcpOwo+ID4gwqAKPiA+IMKgCWlmICh0b19pbnRlbF9lbmNvZGVy KGVuY29kZXIpLT50eXBlICE9IElOVEVMX09VVFBVVF9FRFApCj4gPiDCoAkJcmV0dXJuOwo+ID4g wqAKPiA+IC0JaW50ZWxfZHAgPSBlbmNfdG9faW50ZWxfZHAoZW5jb2Rlcik7Cj4gPiAtCj4gPiDC oAlwcHNfbG9jayhpbnRlbF9kcCk7Cj4gPiDCoAo+ID4gwqAJLyoKCi0tIApWaWxsZSBTeXJqw6Rs w6QKSW50ZWwgT1RDCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9y ZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdm eAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:1972 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751357AbcFVSNK (ORCPT ); Wed, 22 Jun 2016 14:13:10 -0400 Date: Wed, 22 Jun 2016 21:13:05 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Imre Deak Cc: intel-gfx@lists.freedesktop.org, Jani Nikula , stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Refresh cached DP port register value on resume Message-ID: <20160622181305.GE4329@intel.com> References: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> <1466188845.2572.16.camel@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1466188845.2572.16.camel@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Fri, Jun 17, 2016 at 09:40:45PM +0300, Imre Deak wrote: > On Fri, 2016-05-13 at 20:53 +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� > > > > During hibernation the cached DP port register value will be left with > > whatever value we have there when we create the hibernation image. > > Currently that means the port (and eDP PLL) will be off in the cached > > value. However when we resume there is no guarantee that the value > > in the actual register will match the cached value. If i915 isn't > > loaded in the kernel that loads the hibernation image, the port may > > well be on (eg. left on by the BIOS). The encoder state readout > > does the right thing in this case and updates our encoder state > > to reflect the actual hardware state. However the post-resume modeset > > will then use the stale cached port register value in > > intel_dp_link_down() and potentially confuse the hardware. > > > > This was caught by the following assert > > �WARNING: CPU: 3 PID: 5288 at ../drivers/gpu/drm/i915/intel_dp.c:2184 assert_edp_pll+0x99/0xa0 [i915] > > �eDP PLL state assertion failure (expected on, current off) > > on account of the eDP PLL getting prematurely turned off when > > shutting down the port, since the DP_PLL_ENABLE bit wasn't set > > in the cached register value. > > > > Presumably I introduced this problem in > > commit 6fec76628333 ("drm/i915: Use intel_dp->DP in eDP PLL setup") > > as before that we didn't update the cached value after shuttting the > > port down. That's assuming the port got enabled at least once prior > > to hibernating. If that didn't happen then the cached value would > > still have been totally out of sync with reality (eg. first boot w/o > > eDP on, then hibernate, and then resume with eDP on). > > > > So, let's fix this properly and refresh the cached register value from > > the hardware register during resume. > > > > DDI platforms shouldn't use the cached value during port disable at > > least, so shouldn't have this particular issue. They might still have > > issues if we skip the initial modeset and then try to retrain the link > > or something. But untangling this DP vs. DDI mess is a bigger topic, > > so let's jut punt on DDI for now. > > Since the DDI link retraining code seems to reset all relevant parts of > intel_dp->DP (lane, vswing, port enabled) would the above scenario be > really a problem? But syncing intel_dp->DP the same way for DDI makes > sense to me in any case. I didn't look too closely at the DDI case so far, so not sure what's going on there. > > > Cc: Jani Nikula > > Cc: stable@vger.kernel.org > > Fixes: 6fec76628333 ("drm/i915: Use intel_dp->DP in eDP PLL setup") > > Signed-off-by: Ville Syrj�l� > > Reviewed-by: Imre Deak Pushed to dinq. Thanks for the review. > > > --- > > �drivers/gpu/drm/i915/intel_dp.c | 8 +++++--- > > �1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 36330026ceff..a3f38115a3bd 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4522,13 +4522,15 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) > > � > > �void intel_dp_encoder_reset(struct drm_encoder *encoder) > > �{ > > - struct intel_dp *intel_dp; > > + struct drm_i915_private *dev_priv = to_i915(encoder->dev); > > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > + > > + if (!HAS_DDI(dev_priv)) > > + intel_dp->DP = I915_READ(intel_dp->output_reg); > > � > > � if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP) > > � return; > > � > > - intel_dp = enc_to_intel_dp(encoder); > > - > > � pps_lock(intel_dp); > > � > > � /* -- Ville Syrj�l� Intel OTC