From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume Date: Thu, 03 Nov 2016 12:11:55 -0400 Message-ID: <1478189515.28703.9.camel@redhat.com> References: <1478187758-32740-1-git-send-email-lyude@redhat.com> <1478187758-32740-2-git-send-email-lyude@redhat.com> <20161103160253.GA24715@nuc-i3427.alporthouse.com> <1478189469.28703.8.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1478189469.28703.8.camel@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson , Lyude Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAyMDE2LTExLTAzIGF0IDEyOjExIC0wNDAwLCBMeXVkZSBQYXVsIHdyb3RlOgo+IE9u IFRodSwgMjAxNi0xMS0wMyBhdCAxNjowMiArMDAwMCwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+ID4g Cj4gPiBPbiBUaHUsIE5vdiAwMywgMjAxNiBhdCAxMTo0MjozN0FNIC0wNDAwLCBMeXVkZSB3cm90 ZToKPiA+ID4gCj4gPiA+IAo+ID4gPiBXZWluZSdzIGludmVzdGlnYXRpb24gb24gYmVuY2htYXJr aW5nIHRoZSBzdXNwZW5kL3Jlc3VtZSBwcm9jZXNzIHBvaW50ZWQKPiA+ID4gb3V0IGEgbG90IG9m IHRoZSB0aW1lIGluIHN1c3BlbmQvcmVzdW1lIGlzIGJlaW5nIHNwZW50IHJlcHJvYmluZy4gV2hp bGUKPiA+ID4gdGhlIHJlcHJvYmluZyBwcm9jZXNzIGlzIGEgbGVuZ3RoeSBvbmUgZm9yIGdvb2Qg cmVhc29uLCB3ZSBkb24ndCBuZWVkIHRvCj4gPiA+IGhvbGQgdXAgdGhlIGVudGlyZSBzdXNwZW5k L3Jlc3VtZSBwcm9jZXNzIHdoaWxlIHdlIHdhaXQgZm9yIGl0IHRvCj4gPiA+IGZpbmlzaC4gTHVj a2lseSBhcyBpdCB0dXJucyBvdXQsIHdlIGFscmVhZHkgdHJpZ2dlciBhIGZ1bGwgY29ubmVjdG9y Cj4gPiA+IHJlcHJvYmUgaW4gaTkxNV9ocGRfcG9sbF9pbml0X3dvcmsoKSwgc28gd2UgY2FuIGp1 c3QgZGl0Y2ggcmVwcm9iaW5nIGluCj4gPiA+IGk5MTVfZHJtX3Jlc3VtZSgpIGVudGlyZWx5Lgo+ ID4gPiAKPiA+ID4gVGhpcyB3b24ndCBsZWFkIHRvIGxlc3MgdGltZSBzcGVudCByZXN1bWluZyBq dXN0IHlldCBzaW5jZSBub3cgdGhlCj4gPiA+IGJvdHRsZW5lY2sgd2lsbCBiZSB3YWl0aW5nIGZv ciB0aGUgbW9kZV9jb25maWcgbG9jayBpbgo+ID4gPiBkcm1fa21zX2hlbHBlcl9wb2xsX2VuYWJs ZSgpLCBzaW5jZSB0aGF0IHdpbGwgYmUgaGVsZCBhcyBsb25nIGFzCj4gPiA+IGk5MTVfaHBkX3Bv bGxfaW5pdF93b3JrKCkgaXMgcmVwcm9iaW5nIGFsbCBvZiB0aGUgY29ubmVjdG9ycy4gQnV0IHdl J2xsCj4gPiA+IGFkZHJlc3MgdGhhdCBpbiB0aGUgbmV4dCBwYXRjaC4KPiA+ID4gCj4gPiA+IFNp Z25lZC1vZmYtYnk6IEx5dWRlIDxseXVkZUByZWRoYXQuY29tPgo+ID4gPiBDYzogRGF2aWQgV2Vp bmVoYWxsIDxkYXZpZC53ZWluZWhhbGxAbGludXguaW50ZWwuY29tPgo+ID4gPiAtLS0KPiA+ID4g wqBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5jIHwgMiAtLQo+ID4gPiDCoDEgZmlsZSBj aGFuZ2VkLCAyIGRlbGV0aW9ucygtKQo+ID4gPiAKPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv Z3B1L2RybS9pOTE1L2k5MTVfZHJ2LmMKPiA+ID4gYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1 X2Rydi5jCj4gPiA+IGluZGV4IGJmYjJlZmQuLjUzMmNjMGYgMTAwNjQ0Cj4gPiA+IC0tLSBhL2Ry aXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmMKPiA+ID4gKysrIGIvZHJpdmVycy9ncHUvZHJt L2k5MTUvaTkxNV9kcnYuYwo+ID4gPiBAQCAtMTYwMiw4ICsxNjAyLDYgQEAgc3RhdGljIGludCBp OTE1X2RybV9yZXN1bWUoc3RydWN0IGRybV9kZXZpY2UgKmRldikKPiA+ID4gwqAJwqAqIG5vdGlm aWNhdGlvbnMuCj4gPiA+IMKgCcKgKiAqLwo+ID4gPiDCoAlpbnRlbF9ocGRfaW5pdChkZXZfcHJp dik7Cj4gPiA+IC0JLyogQ29uZmlnIG1heSBoYXZlIGNoYW5nZWQgYmV0d2VlbiBzdXNwZW5kIGFu ZCByZXN1bWUgKi8KPiA+ID4gLQlkcm1faGVscGVyX2hwZF9pcnFfZXZlbnQoZGV2KTsKPiA+IAo+ ID4gVGhlIGNvbW1lbnQgaXMgc3RpbGwgYXB0LiBUaGlzIGNvZGUgaXMga25vd24gdG8gYmUgYnJv a2VuIHNpbmNlIGl0Cj4gPiBkb2Vzbid0IGRldGVjdCBhIGNoYW5nZSBpbiBtb25pdG9ycyAoZS5n LiBhIGNoYW5nZSBpbiBleHRlcm5hbCBjb25uZWN0b3JzCj4gPiBmcm9tIGRvY2tpbmcpIGJldHdl ZW4gc3VzcGVuZCBhbmQgcmVzZW5kLiBXZSBzdGlsbCBoYXZlIHRvIHNlbmQgdGhlIHVldmVudC4K PiA+IAo+ID4gKwlkcm1fa21zX2hlbHBlcl9ob3RwbHVnX2V2ZW50KGRldik7Cj4gCj4gSSBtaWdo dCBub3QgaGF2ZSBleHBsYWluZWQgbXlzZWxmIHZlcnkgd2VsbC4gVGhlIHdheSB0aGluZ3Mgc2hv dWxkIGxvb2sgd2l0aAo+IHRoaXMgcGF0Y2ggaXMgbGlrZSB0aGlzOgo+IAo+IGk5MTVfZHJtX3Jl c3VtZSgpCj4gwqAtPiBpbnRlbF9ocGRfaW5pdCgpCj4gwqAgwqAtPiBzZXRzIGRldl9wcml2LT5o b3RwbHVnLnBvbGxfZW5hYmxlZCB0byB0cnVlCldob29wcywgcy90cnVlL2ZhbHNlLwoKPiDCoCDC oC0+IHNjaGVkdWxlcyBkZXZfcHJpdi0+aG90cGx1Zy5wb2xsX2luaXRfd29yawo+IMKgLT4gY29u dGludWUgcmVzdW1l4oCmCj4gCj4gYXQgdGhlIHNhbWUgdGltZToKPiAKPiBpOTE1X2hwZF9wb2xs X2luaXRfd29yaygpIGdldHMgc2NoZWR1bGVkIGFuZCBzdGFydHMKPiDCoC0+IHNpbmNlIGRldl9w cml2LT5ob3RwbHVnLnBvbGxfZW5hYmxlZCA9PSBmYWxzZSwgZHJtX2hlbHBlcl9ocGRfaXJxX2V2 ZW50KCkKPiBpcyBjYWxsZWQKPiDCoCAtPiBkcm1faGVscGVyX2hwZF9pcnFfZXZlbnQoKSByZXBy b2JlcyBjb25uZWN0b3JzCj4gwqDCoMKgLT4gaWYgYW55dGhpbmcgY2hhbmdlZCwgZHJtX2ttc19o ZWxwZXJfaG90cGx1Z19ldmVudCgpIGdldHMgY2FsbGVkLgo+IAo+IFNvIHdlJ3JlIHN0aWxsIHBv bGxpbmcgdGhlIGNvbm5lY3RvcnMgd2hlbiBjb21pbmcgb3V0IG9mIHJlc3VtZSBqdXN0IGxpa2UK PiBiZWZvcmUsIGV4Y2VwdCBub3cgd2UncmUgZG9pbmcgaXQgd2l0aG91dCBuZWVkbGVzc2x5IG1h a2luZyB0aGUgd2hvbGUgcmVzdW1lCj4gcHJvY2VzcyBzdGFsbCB1bnRpbCB3ZSdyZSBkb25lLiBX ZSdyZSBhbHNvIG5vIGxvbmdlciByZXByb2JpbmcgZGlzcGxheQo+IGNvbm5lY3RvcnMgdHdpY2Xi gKYKPiAKPiA+IAo+ID4gCj4gPiB3aGljaCBhbHNvIGRlcGVuZHMgdXBvbiB1cyBhY3R1YWxseSBy ZXNldGluZyB0aGUgY29ubmVjdG9yLT5zdGF0dXMgdG8KPiA+IHVua25vd24gaW4gZHJtX21vZGVf Y29uZmlnX3Jlc2V0KCksIERhbmllbCEKPiA+IC1DaHJpcwo+ID4gCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJp LWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758669AbcKCQL7 (ORCPT ); Thu, 3 Nov 2016 12:11:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54986 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757702AbcKCQL4 (ORCPT ); Thu, 3 Nov 2016 12:11:56 -0400 Message-ID: <1478189515.28703.9.camel@redhat.com> Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume From: Lyude Paul To: Chris Wilson , Lyude Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Date: Thu, 03 Nov 2016 12:11:55 -0400 In-Reply-To: <1478189469.28703.8.camel@redhat.com> References: <1478187758-32740-1-git-send-email-lyude@redhat.com> <1478187758-32740-2-git-send-email-lyude@redhat.com> <20161103160253.GA24715@nuc-i3427.alporthouse.com> <1478189469.28703.8.camel@redhat.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 03 Nov 2016 16:11:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > > out a lot of the time in suspend/resume is being spent reprobing. While > > > the reprobing process is a lengthy one for good reason, we don't need to > > > hold up the entire suspend/resume process while we wait for it to > > > finish. Luckily as it turns out, we already trigger a full connector > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > > i915_drm_resume() entirely. > > > > > > This won't lead to less time spent resuming just yet since now the > > > bottleneck will be waiting for the mode_config lock in > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > > address that in the next patch. > > > > > > Signed-off-by: Lyude > > > Cc: David Weinehall > > > --- > > >  drivers/gpu/drm/i915/i915_drv.c | 2 -- > > >  1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index bfb2efd..532cc0f 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > >    * notifications. > > >    * */ > > >   intel_hpd_init(dev_priv); > > > - /* Config may have changed between suspend and resume */ > > > - drm_helper_hpd_irq_event(dev); > > > > The comment is still apt. This code is known to be broken since it > > doesn't detect a change in monitors (e.g. a change in external connectors > > from docking) between suspend and resend. We still have to send the uevent. > > > > + drm_kms_helper_hotplug_event(dev); > > I might not have explained myself very well. The way things should look with > this patch is like this: > > i915_drm_resume() >  -> intel_hpd_init() >    -> sets dev_priv->hotplug.poll_enabled to true Whoops, s/true/false/ >    -> schedules dev_priv->hotplug.poll_init_work >  -> continue resume… > > at the same time: > > i915_hpd_poll_init_work() gets scheduled and starts >  -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() > is called >   -> drm_helper_hpd_irq_event() reprobes connectors >    -> if anything changed, drm_kms_helper_hotplug_event() gets called. > > So we're still polling the connectors when coming out of resume just like > before, except now we're doing it without needlessly making the whole resume > process stall until we're done. We're also no longer reprobing display > connectors twice… > > > > > > > which also depends upon us actually reseting the connector->status to > > unknown in drm_mode_config_reset(), Daniel! > > -Chris > >