From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH] drm/i915: Refresh cached DP port register value on resume Date: Fri, 17 Jun 2016 21:40:45 +0300 Message-ID: <1466188845.2572.16.camel@intel.com> References: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 371C36EC9F for ; Fri, 17 Jun 2016 18:40:53 +0000 (UTC) In-Reply-To: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Jani Nikula , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gRnJpLCAyMDE2LTA1LTEzIGF0IDIwOjUzICswMzAwLCB2aWxsZS5zeXJqYWxhQGxpbnV4Lmlu dGVsLmNvbSB3cm90ZToKPiBGcm9tOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGlu dXguaW50ZWwuY29tPgo+IAo+IER1cmluZyBoaWJlcm5hdGlvbiB0aGUgY2FjaGVkIERQIHBvcnQg cmVnaXN0ZXIgdmFsdWUgd2lsbCBiZSBsZWZ0IHdpdGgKPiB3aGF0ZXZlciB2YWx1ZSB3ZSBoYXZl IHRoZXJlIHdoZW4gd2UgY3JlYXRlIHRoZSBoaWJlcm5hdGlvbiBpbWFnZS4KPiBDdXJyZW50bHkg dGhhdCBtZWFucyB0aGUgcG9ydCAoYW5kIGVEUCBQTEwpIHdpbGwgYmUgb2ZmIGluIHRoZSBjYWNo ZWQKPiB2YWx1ZS4gSG93ZXZlciB3aGVuIHdlIHJlc3VtZSB0aGVyZSBpcyBubyBndWFyYW50ZWUg dGhhdCB0aGUgdmFsdWUKPiBpbiB0aGUgYWN0dWFsIHJlZ2lzdGVyIHdpbGwgbWF0Y2ggdGhlIGNh Y2hlZCB2YWx1ZS4gSWYgaTkxNSBpc24ndAo+IGxvYWRlZCBpbiB0aGUga2VybmVsIHRoYXQgbG9h ZHMgdGhlIGhpYmVybmF0aW9uIGltYWdlLCB0aGUgcG9ydCBtYXkKPiB3ZWxsIGJlIG9uIChlZy4g bGVmdCBvbiBieSB0aGUgQklPUykuIFRoZSBlbmNvZGVyIHN0YXRlIHJlYWRvdXQKPiBkb2VzIHRo ZSByaWdodCB0aGluZyBpbiB0aGlzIGNhc2UgYW5kIHVwZGF0ZXMgb3VyIGVuY29kZXIgc3RhdGUK PiB0byByZWZsZWN0IHRoZSBhY3R1YWwgaGFyZHdhcmUgc3RhdGUuIEhvd2V2ZXIgdGhlIHBvc3Qt cmVzdW1lIG1vZGVzZXQKPiB3aWxsIHRoZW4gdXNlIHRoZSBzdGFsZSBjYWNoZWQgcG9ydCByZWdp c3RlciB2YWx1ZSBpbgo+IGludGVsX2RwX2xpbmtfZG93bigpIGFuZCBwb3RlbnRpYWxseSBjb25m dXNlIHRoZSBoYXJkd2FyZS4KPiAKPiBUaGlzIHdhcyBjYXVnaHQgYnkgdGhlIGZvbGxvd2luZyBh c3NlcnQKPiDCoFdBUk5JTkc6IENQVTogMyBQSUQ6IDUyODggYXQgLi4vZHJpdmVycy9ncHUvZHJt L2k5MTUvaW50ZWxfZHAuYzoyMTg0IGFzc2VydF9lZHBfcGxsKzB4OTkvMHhhMCBbaTkxNV0KPiDC oGVEUCBQTEwgc3RhdGUgYXNzZXJ0aW9uIGZhaWx1cmUgKGV4cGVjdGVkIG9uLCBjdXJyZW50IG9m ZikKPiBvbiBhY2NvdW50IG9mIHRoZSBlRFAgUExMIGdldHRpbmcgcHJlbWF0dXJlbHkgdHVybmVk IG9mZiB3aGVuCj4gc2h1dHRpbmcgZG93biB0aGUgcG9ydCwgc2luY2UgdGhlIERQX1BMTF9FTkFC TEUgYml0IHdhc24ndCBzZXQKPiBpbiB0aGUgY2FjaGVkIHJlZ2lzdGVyIHZhbHVlLgo+IAo+IFBy ZXN1bWFibHkgSSBpbnRyb2R1Y2VkIHRoaXMgcHJvYmxlbSBpbgo+IGNvbW1pdCA2ZmVjNzY2Mjgz MzMgKCJkcm0vaTkxNTogVXNlIGludGVsX2RwLT5EUCBpbiBlRFAgUExMIHNldHVwIikKPiBhcyBi ZWZvcmUgdGhhdCB3ZSBkaWRuJ3QgdXBkYXRlIHRoZSBjYWNoZWQgdmFsdWUgYWZ0ZXIgc2h1dHR0 aW5nIHRoZQo+IHBvcnQgZG93bi4gVGhhdCdzIGFzc3VtaW5nIHRoZSBwb3J0IGdvdCBlbmFibGVk IGF0IGxlYXN0IG9uY2UgcHJpb3IKPiB0byBoaWJlcm5hdGluZy4gSWYgdGhhdCBkaWRuJ3QgaGFw cGVuIHRoZW4gdGhlIGNhY2hlZCB2YWx1ZSB3b3VsZAo+IHN0aWxsIGhhdmUgYmVlbiB0b3RhbGx5 IG91dCBvZiBzeW5jIHdpdGggcmVhbGl0eSAoZWcuIGZpcnN0IGJvb3Qgdy9vCj4gZURQIG9uLCB0 aGVuIGhpYmVybmF0ZSwgYW5kIHRoZW4gcmVzdW1lIHdpdGggZURQIG9uKS4KPiAKPiBTbywgbGV0 J3MgZml4IHRoaXMgcHJvcGVybHkgYW5kIHJlZnJlc2ggdGhlIGNhY2hlZCByZWdpc3RlciB2YWx1 ZSBmcm9tCj4gdGhlIGhhcmR3YXJlIHJlZ2lzdGVyIGR1cmluZyByZXN1bWUuCj4gCj4gRERJIHBs YXRmb3JtcyBzaG91bGRuJ3QgdXNlIHRoZSBjYWNoZWQgdmFsdWUgZHVyaW5nIHBvcnQgZGlzYWJs ZSBhdAo+IGxlYXN0LCBzbyBzaG91bGRuJ3QgaGF2ZSB0aGlzIHBhcnRpY3VsYXIgaXNzdWUuIFRo ZXkgbWlnaHQgc3RpbGwgaGF2ZQo+IGlzc3VlcyBpZiB3ZSBza2lwIHRoZSBpbml0aWFsIG1vZGVz ZXQgYW5kIHRoZW4gdHJ5IHRvIHJldHJhaW4gdGhlIGxpbmsKPiBvciBzb21ldGhpbmcuIEJ1dCB1 bnRhbmdsaW5nIHRoaXMgRFAgdnMuIERESSBtZXNzIGlzIGEgYmlnZ2VyIHRvcGljLAo+IHNvIGxl dCdzIGp1dCBwdW50IG9uIERESSBmb3Igbm93LgoKU2luY2UgdGhlIERESSBsaW5rIHJldHJhaW5p bmcgY29kZSBzZWVtcyB0byByZXNldCBhbGwgcmVsZXZhbnQgcGFydHMgb2YKaW50ZWxfZHAtPkRQ IChsYW5lLCB2c3dpbmcsIHBvcnQgZW5hYmxlZCkgd291bGQgdGhlIGFib3ZlIHNjZW5hcmlvIGJl CnJlYWxseSBhIHByb2JsZW0/IEJ1dCBzeW5jaW5nIGludGVsX2RwLT5EUCB0aGUgc2FtZSB3YXkg Zm9yIERESSBtYWtlcwpzZW5zZSB0byBtZSBpbiBhbnkgY2FzZS4KCj4gQ2M6IEphbmkgTmlrdWxh IDxqYW5pLm5pa3VsYUBpbnRlbC5jb20+Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiBG aXhlczogNmZlYzc2NjI4MzMzICgiZHJtL2k5MTU6IFVzZSBpbnRlbF9kcC0+RFAgaW4gZURQIFBM TCBzZXR1cCIpCj4gU2lnbmVkLW9mZi1ieTogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxh QGxpbnV4LmludGVsLmNvbT4KClJldmlld2VkLWJ5OiBJbXJlIERlYWsgPGltcmUuZGVha0BpbnRl bC5jb20+Cgo+IC0tLQo+IMKgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYyB8IDggKysr KystLS0KPiDCoDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0p Cj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMgYi9kcml2 ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcC5jCj4gaW5kZXggMzYzMzAwMjZjZWZmLi5hM2YzODEx NWEzYmQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHAuYwo+ICsr KyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwLmMKPiBAQCAtNDUyMiwxMyArNDUyMiwx NSBAQCBzdGF0aWMgdm9pZCBpbnRlbF9lZHBfcGFuZWxfdmRkX3Nhbml0aXplKHN0cnVjdCBpbnRl bF9kcCAqaW50ZWxfZHApCj4gwqAKPiDCoHZvaWQgaW50ZWxfZHBfZW5jb2Rlcl9yZXNldChzdHJ1 Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIpCj4gwqB7Cj4gLQlzdHJ1Y3QgaW50ZWxfZHAgKmludGVs X2RwOwo+ICsJc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2ID0gdG9faTkxNShlbmNv ZGVyLT5kZXYpOwo+ICsJc3RydWN0IGludGVsX2RwICppbnRlbF9kcCA9IGVuY190b19pbnRlbF9k cChlbmNvZGVyKTsKPiArCj4gKwlpZiAoIUhBU19EREkoZGV2X3ByaXYpKQo+ICsJCWludGVsX2Rw LT5EUCA9IEk5MTVfUkVBRChpbnRlbF9kcC0+b3V0cHV0X3JlZyk7Cj4gwqAKPiDCoAlpZiAodG9f aW50ZWxfZW5jb2RlcihlbmNvZGVyKS0+dHlwZSAhPSBJTlRFTF9PVVRQVVRfRURQKQo+IMKgCQly ZXR1cm47Cj4gwqAKPiAtCWludGVsX2RwID0gZW5jX3RvX2ludGVsX2RwKGVuY29kZXIpOwo+IC0K PiDCoAlwcHNfbG9jayhpbnRlbF9kcCk7Cj4gwqAKPiDCoAkvKgpfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVs LWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:58252 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751134AbcFQSkx (ORCPT ); Fri, 17 Jun 2016 14:40:53 -0400 Message-ID: <1466188845.2572.16.camel@intel.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Refresh cached DP port register value on resume From: Imre Deak Reply-To: imre.deak@intel.com To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Jani Nikula , stable@vger.kernel.org Date: Fri, 17 Jun 2016 21:40:45 +0300 In-Reply-To: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> References: <1463162036-27931-1-git-send-email-ville.syrjala@linux.intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: 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. > 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 > --- >  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); >   >   /*