From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Thulasimani, Sivakumar" Subject: Re: [PATCH] drm/i915: Resume DP MST before doing any kind of modesetting Date: Wed, 24 Feb 2016 08:03:04 +0530 Message-ID: <56CD1660.2030203@intel.com> References: <1456265512-15670-1-git-send-email-cpaul@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1456265512-15670-1-git-send-email-cpaul@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Lyude , intel-gfx@lists.freedesktop.org Cc: David Airlie , Daniel Vetter , "open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list" , stable@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org CgpPbiAyLzI0LzIwMTYgMzo0MSBBTSwgTHl1ZGUgd3JvdGU6Cj4gQXMgaXQgdHVybnMgb3V0LCBy ZXN1bWluZyBEUCBNU1QgaXMgcmFjZXkgc2luY2Ugd2UgZG9uJ3QgbWFrZSBzdXJlIE1TVAo+IGlz IHJlYWR5IGJlZm9yZSB3ZSBzdGFydCBtb2Rlc2V0dGluZywgaXQganVzdCB1c3VhbGx5IGhhcHBl bnMgdG8gYmUKPiByZWFkeSBpbiB0aW1lLiBUaGlzIGlzbid0IHRoZSBjYXNlIG9uIGFsbCBzeXN0 ZW1zLCBwYXJ0aWN1bGFybHkgYQo+IFRoaW5rUGFkIFQ1NjAgd2l0aCBkaXNwbGF5cyBjb25uZWN0 ZWQgdGhyb3VnaCB0aGUgZG9jay4gT24gdGhlc2UKPiBzeXN0ZW1zLCByZXN1bWluZyB0aGUgbGFw dG9wIHdoaWxlIGNvbm5lY3RlZCB0byB0aGUgZG9jayB1c3VhbGx5IHJlc3VsdHMKPiBpbiBibGFu ayBtb25pdG9ycy4gTWFraW5nIHN1cmUgTVNUIGlzIHJlYWR5IGJlZm9yZSBkb2luZyBhbnkga2lu ZCBvZgo+IG1vZGVzZXR0aW5nIGZpeGVzIHRoaXMgaXNzdWUuCmJhc2ljIHF1ZXN0aW9uIHNpbmNl IGkgaGF2ZW4ndCB3b3JrZWQgb24gTVNUIG11Y2guIE1TVCBzaG91bGQgd29yayBsaWtlIGFueQpv dGhlciBkaWdpdGFsIHBhbmVsIG9uIHJlc3VtZS4gaS5lIGRldGVjdCBmb2xsb3dlZCBieSBtb2Rl c2V0LiBpbiB0aGUgCm1vZGlmaWVkCmNvbW1pdCBtZW50aW9uZWQgYmVsb3cgaXMgaXQgZmFpbGlu ZyB0byBkZXRlY3QgdGhlIHBhbmVsIG9yIGZhaWxpbmcgYXQgCnRoZSBtb2Rlc2V0ID8KaWYgd2Ug YXJlIGRlcGVuZGluZyBvbiB0aGUgaW50ZWxfZGlzcGxheV9yZXN1bWUsIGhvdyBhYm91dCBtb3Zp bmcgdGhlCmludGVsX2RwX21zdF9yZXN1bWUganVzdCBhYm92ZSBpbnRlbF9kaXNwbGF5X3Jlc3Vt ZT8KCgpHZW5lcmljIHF1ZXN0aW9uIHRvIG90aGVycyBpbiBtYWlsIGxpc3Qgb24gaTkxNV9kcm1f cmVzdW1lCndlIGFyZSBkb2luZyBhIG1vZGVzZXQgYW5kIHRoZW4gZG9pbmcgdGhlIGRldGVjdC9o cGQgaW5pdC4Kc2hvdWxkbid0IHRoaXMgYmUgdGhlIG90aGVyIHdheSByb3VuZCA/IGFsbW9zdCBh bGwgZGlzcGxheXMKd2lsbCBwYXNzIGEgbW9kZXNldCBldmVuIGlmIGRpc3BsYXkgaXMgbm90IGNv bm5lY3RlZCBzbyB3ZQphcmUgc3BlbmRpbmcgdGltZSBvbiBtb2Rlc2V0IGV2ZW4gZm9yIGRpc3Bs YXlzIHRoYXQgd2VyZQpyZW1vdmVkIGR1cmluZyB0aGUgc3VzcGVuZCBzdGF0ZS4gaWYgdGhpcyBp cyB0byBzaW1wbHkKZHJtX3N0YXRlIGJlaW5nIHNhdmVkIGFuZCByZXN0b3JlZCwKPiBXZSBvcmln aW5hbGx5IGNoYW5nZWQgdGhlIHJlc3VtZSBvcmRlciBpbgo+Cj4gCWNvbW1pdCBlN2Q2ZjdkNzA4 MjkgKCJkcm0vaTkxNTogcmVzdW1lIE1TVCBhZnRlciByZWFkaW5nIGJhY2sgaHcgc3RhdGUiKQo+ Cj4gdG8gZml4IGEgdG9uIG9mIFdBUk5fT04ncyBhZnRlciByZXN1bWUsIGJ1dCB0aGlzIGRvZXNu J3Qgc2VlbSB0byBiZSBhbgo+IGlzc3VlIG5vdyBhbnlob3cuCj4KPiBDQzogc3RhYmxlQHZnZXIu a2VybmVsLm9yZwo+IFNpZ25lZC1vZmYtYnk6IEx5dWRlIDxjcGF1bEByZWRoYXQuY29tPgo+IC0t LQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuYyB8IDEwICsrKysrKysrLS0KPiAg IDEgZmlsZSBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4KPiBkaWZm IC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuYyBiL2RyaXZlcnMvZ3B1L2Ry bS9pOTE1L2k5MTVfZHJ2LmMKPiBpbmRleCBmMzU3MDU4Li40ZGNmM2RkIDEwMDY0NAo+IC0tLSBh L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pOTE1X2Rydi5jCj4gQEAgLTczMyw2ICs3MzMsMTQgQEAgc3RhdGljIGludCBpOTE1X2Ry bV9yZXN1bWUoc3RydWN0IGRybV9kZXZpY2UgKmRldikKPiAgIAlpbnRlbF9vcHJlZ2lvbl9zZXR1 cChkZXYpOwo+ICAgCj4gICAJaW50ZWxfaW5pdF9wY2hfcmVmY2xrKGRldik7Cj4gKwo+ICsJLyoK PiArCSAqIFdlIG5lZWQgdG8gbWFrZSBzdXJlIHRoYXQgd2UgcmVzdW1lIE1TVCBiZWZvcmUgZG9p bmcgYW55dGhpbmcKPiArCSAqIGRpc3BsYXkgcmVsYXRlZCwgb3RoZXJ3aXNlIHdlIHJpc2sgdHJ5 aW5nIHRvIGJyaW5nIHVwIGEgZGlzcGxheSBvbgo+ICsJICogTVNUIGJlZm9yZSB0aGUgaHViIGlz IGFjdHVhbGx5IHJlYWR5Cj4gKwkgKi8KPiArCWludGVsX2RwX21zdF9yZXN1bWUoZGV2KTsKPiAr ClRoaXMgZG9lcyBub3QgbG9vayBwcm9wZXIuIGlmIHRoZSBDRCBjbG9jayBpcyB0dXJuZWQgb2Zm IGR1cmluZyBzdXNwZW5kCm91ciBkcGNkIHJlYWQgaXRzZWxmIG1pZ2h0IGZhaWwgdGlsbCB3ZSBl bmFibGUgQ0QgQ2xvY2suCgpyZWdhcmRzLApTaXZha3VtYXIKPiAgIAlkcm1fbW9kZV9jb25maWdf cmVzZXQoZGV2KTsKPiAgIAo+ICAgCS8qCj4gQEAgLTc2NSw4ICs3NzMsNiBAQCBzdGF0aWMgaW50 IGk5MTVfZHJtX3Jlc3VtZShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQo+ICAgCWludGVsX2Rpc3Bs YXlfcmVzdW1lKGRldik7Cj4gICAJZHJtX21vZGVzZXRfdW5sb2NrX2FsbChkZXYpOwo+ICAgCj4g LQlpbnRlbF9kcF9tc3RfcmVzdW1lKGRldik7Cj4gLQo+ICAgCS8qCj4gICAJICogLi4uIGJ1dCBh bHNvIG5lZWQgdG8gbWFrZSBzdXJlIHRoYXQgaG90cGx1ZyBwcm9jZXNzaW5nCj4gICAJICogZG9l c24ndCBjYXVzZSBoYXZvYy4gTGlrZSBpbiB0aGUgZHJpdmVyIGxvYWQgY29kZSB3ZSBkb24ndAoK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:31965 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753166AbcBXCdf (ORCPT ); Tue, 23 Feb 2016 21:33:35 -0500 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Resume DP MST before doing any kind of modesetting To: Lyude , intel-gfx@lists.freedesktop.org References: <1456265512-15670-1-git-send-email-cpaul@redhat.com> Cc: David Airlie , stable@vger.kernel.org, open "list:INTEL" DRM DRIVERS excluding "Poulsbo," "Moorestow...," "linux-kernel@vger.kernel.org" open list , Daniel Vetter From: "Thulasimani, Sivakumar" Message-ID: <56CD1660.2030203@intel.com> Date: Wed, 24 Feb 2016 08:03:04 +0530 MIME-Version: 1.0 In-Reply-To: <1456265512-15670-1-git-send-email-cpaul@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 2/24/2016 3:41 AM, Lyude wrote: > As it turns out, resuming DP MST is racey since we don't make sure MST > is ready before we start modesetting, it just usually happens to be > ready in time. This isn't the case on all systems, particularly a > ThinkPad T560 with displays connected through the dock. On these > systems, resuming the laptop while connected to the dock usually results > in blank monitors. Making sure MST is ready before doing any kind of > modesetting fixes this issue. basic question since i haven't worked on MST much. MST should work like any other digital panel on resume. i.e detect followed by modeset. in the modified commit mentioned below is it failing to detect the panel or failing at the modeset ? if we are depending on the intel_display_resume, how about moving the intel_dp_mst_resume just above intel_display_resume? Generic question to others in mail list on i915_drm_resume we are doing a modeset and then doing the detect/hpd init. shouldn't this be the other way round ? almost all displays will pass a modeset even if display is not connected so we are spending time on modeset even for displays that were removed during the suspend state. if this is to simply drm_state being saved and restored, > We originally changed the resume order in > > commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") > > to fix a ton of WARN_ON's after resume, but this doesn't seem to be an > issue now anyhow. > > CC: stable@vger.kernel.org > Signed-off-by: Lyude > --- > drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index f357058..4dcf3dd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -733,6 +733,14 @@ static int i915_drm_resume(struct drm_device *dev) > intel_opregion_setup(dev); > > intel_init_pch_refclk(dev); > + > + /* > + * We need to make sure that we resume MST before doing anything > + * display related, otherwise we risk trying to bring up a display on > + * MST before the hub is actually ready > + */ > + intel_dp_mst_resume(dev); > + This does not look proper. if the CD clock is turned off during suspend our dpcd read itself might fail till we enable CD Clock. regards, Sivakumar > drm_mode_config_reset(dev); > > /* > @@ -765,8 +773,6 @@ static int i915_drm_resume(struct drm_device *dev) > intel_display_resume(dev); > drm_modeset_unlock_all(dev); > > - intel_dp_mst_resume(dev); > - > /* > * ... but also need to make sure that hotplug processing > * doesn't cause havoc. Like in the driver load code we don't