From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934221AbeBMPqM (ORCPT ); Tue, 13 Feb 2018 10:46:12 -0500 Received: from foss.arm.com ([217.140.101.70]:59646 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751465AbeBMPqL (ORCPT ); Tue, 13 Feb 2018 10:46:11 -0500 Date: Tue, 13 Feb 2018 15:46:08 +0000 From: Liviu Dudau To: Lukas Wunner Cc: Tejun Heo , Lai Jiangshan , Alex Deucher , Dave Airlie , Ben Skeggs , dri-devel@lists.freedesktop.org, Peter Wu , nouveau@lists.freedesktop.org, Lyude Paul , Hans de Goede , Pierre Moreau , linux-kernel@vger.kernel.org, Ismo Toijala , intel-gfx@lists.freedesktop.org, Archit Taneja Subject: Re: [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers Message-ID: <20180213154608.GI9111@e110455-lin.cambridge.arm.com> References: <20180213105506.GF9111@e110455-lin.cambridge.arm.com> <20180213115206.GA16075@wunner.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180213115206.GA16075@wunner.de> User-Agent: Mutt/1.9.3 (2018-01-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 13, 2018 at 12:52:06PM +0100, Lukas Wunner wrote: > On Tue, Feb 13, 2018 at 10:55:06AM +0000, Liviu Dudau wrote: > > On Sun, Feb 11, 2018 at 10:38:28AM +0100, Lukas Wunner wrote: > > > DRM drivers poll connectors in 10 sec intervals. The poll worker is > > > stopped on ->runtime_suspend with cancel_delayed_work_sync(). However > > > the poll worker invokes the DRM drivers' ->detect callbacks, which call > > > pm_runtime_get_sync(). If the poll worker starts after runtime suspend > > > has begun, pm_runtime_get_sync() will wait for runtime suspend to finish > > > with the intention of runtime resuming the device afterwards. The result > > > is a circular wait between poll worker and autosuspend worker. > > > > I think I understand the problem you are trying to solve, but I'm > > struggling to understand where malidp makes any specific mistakes. First > > of all, malidp is only a display engine, so there is no GPU attached to > > it, but that is only a small clarification. Second, malidp doesn't use > > directly any of the callbacks that you are referring to, it uses the > > drm_cma_xxxx() API plus the generic drm_xxxx() call. So if there are any > > issues there (as they might well be) I think they would apply to a lot > > more drivers and the fix will involve more than just malidp, i915 and > > msm. > > I don't know if malidp makes any specific mistakes and didn't mean to > cast it in a negative light, sorry. I did not take what you've said as a negative thing, only wanted to understand how you came to your conclusions. > > So a lot of DRM drivers acquire a runtime PM ref in the connector ->detect > hook because they can't probe connectors while runtime suspended. > E.g. for a PCI device, probing might require mmio access, which isn't > possible outside of power state D0. There are no ->detect hooks declared > in drivers/gpu/drm/arm/, so it's unclear to me whether you're able to probe > during runtime suspend. That's because the drivers in drivers/gpu/drm/arm do not have connectors, they are only the CRTC part of the driver. Both hdlcd and mali-dp use the component framework to locate an encoder in device tree that will then provide the connectors. > > hdlcd_drv.c and malidp_drv.c both enable output polling. Output polling > is only necessary if you don't get HPD interrupts. That's right, hdlcd and mali-dp don't receive HPD interrupts because they don't have any. And because we don't know ahead of time which encoder/connector will be paired with the driver, we enable polling as a safe fallback. > > You're not disabling polling upon runtime suspend. Thus, if a runtime PM > ref is acquired during polling (such as in a ->detect hook), the GPU will > be runtime resumed every 10 secs. You may want to verify that's not the > case. If you decide that you do want to stop polling during runtime > suspend because it runtime resumes the GPU continuously, you'll need the > helper introduced in this series. So by cc'ing you I just wanted to keep > you in the loop about an issue that may potentially affect your driver. Again, we have no GPU linked to us and the polling during runtime suspend should be handled by the driver for the paired encoder, not by us. I understand the potential issue but I'm struggling to understand if it really applies to the drivers/gpu/drm/arm drivers other than in an abstract way. Best regards, Liviu > > Let me know if there are any questions. > > Thanks, > > Lukas -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH 0/5] Fix deadlock on runtime suspend in DRM drivers Date: Tue, 13 Feb 2018 15:46:08 +0000 Message-ID: <20180213154608.GI9111@e110455-lin.cambridge.arm.com> References: <20180213105506.GF9111@e110455-lin.cambridge.arm.com> <20180213115206.GA16075@wunner.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20180213115206.GA16075@wunner.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Lukas Wunner Cc: Pierre Moreau , Archit Taneja , Ismo Toijala , Hans de Goede , nouveau@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Lai Jiangshan , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Alex Deucher , Ben Skeggs , Tejun Heo , Dave Airlie , Peter Wu List-Id: nouveau.vger.kernel.org T24gVHVlLCBGZWIgMTMsIDIwMTggYXQgMTI6NTI6MDZQTSArMDEwMCwgTHVrYXMgV3VubmVyIHdy b3RlOgo+IE9uIFR1ZSwgRmViIDEzLCAyMDE4IGF0IDEwOjU1OjA2QU0gKzAwMDAsIExpdml1IER1 ZGF1IHdyb3RlOgo+ID4gT24gU3VuLCBGZWIgMTEsIDIwMTggYXQgMTA6Mzg6MjhBTSArMDEwMCwg THVrYXMgV3VubmVyIHdyb3RlOgo+ID4gPiBEUk0gZHJpdmVycyBwb2xsIGNvbm5lY3RvcnMgaW4g MTAgc2VjIGludGVydmFscy4gIFRoZSBwb2xsIHdvcmtlciBpcwo+ID4gPiBzdG9wcGVkIG9uIC0+ cnVudGltZV9zdXNwZW5kIHdpdGggY2FuY2VsX2RlbGF5ZWRfd29ya19zeW5jKCkuICBIb3dldmVy Cj4gPiA+IHRoZSBwb2xsIHdvcmtlciBpbnZva2VzIHRoZSBEUk0gZHJpdmVycycgLT5kZXRlY3Qg Y2FsbGJhY2tzLCB3aGljaCBjYWxsCj4gPiA+IHBtX3J1bnRpbWVfZ2V0X3N5bmMoKS4gIElmIHRo ZSBwb2xsIHdvcmtlciBzdGFydHMgYWZ0ZXIgcnVudGltZSBzdXNwZW5kCj4gPiA+IGhhcyBiZWd1 biwgcG1fcnVudGltZV9nZXRfc3luYygpIHdpbGwgd2FpdCBmb3IgcnVudGltZSBzdXNwZW5kIHRv IGZpbmlzaAo+ID4gPiB3aXRoIHRoZSBpbnRlbnRpb24gb2YgcnVudGltZSByZXN1bWluZyB0aGUg ZGV2aWNlIGFmdGVyd2FyZHMuICBUaGUgcmVzdWx0Cj4gPiA+IGlzIGEgY2lyY3VsYXIgd2FpdCBi ZXR3ZWVuIHBvbGwgd29ya2VyIGFuZCBhdXRvc3VzcGVuZCB3b3JrZXIuCj4gPiAKPiA+IEkgdGhp bmsgSSB1bmRlcnN0YW5kIHRoZSBwcm9ibGVtIHlvdSBhcmUgdHJ5aW5nIHRvIHNvbHZlLCBidXQg SSdtCj4gPiBzdHJ1Z2dsaW5nIHRvIHVuZGVyc3RhbmQgd2hlcmUgbWFsaWRwIG1ha2VzIGFueSBz cGVjaWZpYyBtaXN0YWtlcy4gRmlyc3QKPiA+IG9mIGFsbCwgbWFsaWRwIGlzIG9ubHkgYSBkaXNw bGF5IGVuZ2luZSwgc28gdGhlcmUgaXMgbm8gR1BVIGF0dGFjaGVkIHRvCj4gPiBpdCwgYnV0IHRo YXQgaXMgb25seSBhIHNtYWxsIGNsYXJpZmljYXRpb24uIFNlY29uZCwgbWFsaWRwIGRvZXNuJ3Qg dXNlCj4gPiBkaXJlY3RseSBhbnkgb2YgdGhlIGNhbGxiYWNrcyB0aGF0IHlvdSBhcmUgcmVmZXJy aW5nIHRvLCBpdCB1c2VzIHRoZQo+ID4gZHJtX2NtYV94eHh4KCkgQVBJIHBsdXMgdGhlIGdlbmVy aWMgZHJtX3h4eHgoKSBjYWxsLiBTbyBpZiB0aGVyZSBhcmUgYW55Cj4gPiBpc3N1ZXMgdGhlcmUg KGFzIHRoZXkgbWlnaHQgd2VsbCBiZSkgSSB0aGluayB0aGV5IHdvdWxkIGFwcGx5IHRvIGEgbG90 Cj4gPiBtb3JlIGRyaXZlcnMgYW5kIHRoZSBmaXggd2lsbCBpbnZvbHZlIG1vcmUgdGhhbiBqdXN0 IG1hbGlkcCwgaTkxNSBhbmQKPiA+IG1zbS4KPiAKPiBJIGRvbid0IGtub3cgaWYgbWFsaWRwIG1h a2VzIGFueSBzcGVjaWZpYyBtaXN0YWtlcyBhbmQgZGlkbid0IG1lYW4gdG8KPiBjYXN0IGl0IGlu IGEgbmVnYXRpdmUgbGlnaHQsIHNvcnJ5LgoKSSBkaWQgbm90IHRha2Ugd2hhdCB5b3UndmUgc2Fp ZCBhcyBhIG5lZ2F0aXZlIHRoaW5nLCBvbmx5IHdhbnRlZCB0bwp1bmRlcnN0YW5kIGhvdyB5b3Ug Y2FtZSB0byB5b3VyIGNvbmNsdXNpb25zLgoKPiAKPiBTbyBhIGxvdCBvZiBEUk0gZHJpdmVycyBh Y3F1aXJlIGEgcnVudGltZSBQTSByZWYgaW4gdGhlIGNvbm5lY3RvciAtPmRldGVjdAo+IGhvb2sg YmVjYXVzZSB0aGV5IGNhbid0IHByb2JlIGNvbm5lY3RvcnMgd2hpbGUgcnVudGltZSBzdXNwZW5k ZWQuCj4gRS5nLiBmb3IgYSBQQ0kgZGV2aWNlLCBwcm9iaW5nIG1pZ2h0IHJlcXVpcmUgbW1pbyBh Y2Nlc3MsIHdoaWNoIGlzbid0Cj4gcG9zc2libGUgb3V0c2lkZSBvZiBwb3dlciBzdGF0ZSBEMC4g IFRoZXJlIGFyZSBubyAtPmRldGVjdCBob29rcyBkZWNsYXJlZAo+IGluIGRyaXZlcnMvZ3B1L2Ry bS9hcm0vLCBzbyBpdCdzIHVuY2xlYXIgdG8gbWUgd2hldGhlciB5b3UncmUgYWJsZSB0byBwcm9i ZQo+IGR1cmluZyBydW50aW1lIHN1c3BlbmQuCgpUaGF0J3MgYmVjYXVzZSB0aGUgZHJpdmVycyBp biBkcml2ZXJzL2dwdS9kcm0vYXJtIGRvIG5vdCBoYXZlCmNvbm5lY3RvcnMsIHRoZXkgYXJlIG9u bHkgdGhlIENSVEMgcGFydCBvZiB0aGUgZHJpdmVyLiBCb3RoIGhkbGNkIGFuZAptYWxpLWRwIHVz ZSB0aGUgY29tcG9uZW50IGZyYW1ld29yayB0byBsb2NhdGUgYW4gZW5jb2RlciBpbiBkZXZpY2Ug dHJlZQp0aGF0IHdpbGwgdGhlbiBwcm92aWRlIHRoZSBjb25uZWN0b3JzLgoKPiAKPiBoZGxjZF9k cnYuYyBhbmQgbWFsaWRwX2Rydi5jIGJvdGggZW5hYmxlIG91dHB1dCBwb2xsaW5nLiAgT3V0cHV0 IHBvbGxpbmcKPiBpcyBvbmx5IG5lY2Vzc2FyeSBpZiB5b3UgZG9uJ3QgZ2V0IEhQRCBpbnRlcnJ1 cHRzLgoKVGhhdCdzIHJpZ2h0LCBoZGxjZCBhbmQgbWFsaS1kcCBkb24ndCByZWNlaXZlIEhQRCBp bnRlcnJ1cHRzIGJlY2F1c2UKdGhleSBkb24ndCBoYXZlIGFueS4gQW5kIGJlY2F1c2Ugd2UgZG9u J3Qga25vdyBhaGVhZCBvZiB0aW1lIHdoaWNoCmVuY29kZXIvY29ubmVjdG9yIHdpbGwgYmUgcGFp cmVkIHdpdGggdGhlIGRyaXZlciwgd2UgZW5hYmxlIHBvbGxpbmcgYXMgYQpzYWZlIGZhbGxiYWNr LgoKPiAKPiBZb3UncmUgbm90IGRpc2FibGluZyBwb2xsaW5nIHVwb24gcnVudGltZSBzdXNwZW5k LiAgVGh1cywgaWYgYSBydW50aW1lIFBNCj4gcmVmIGlzIGFjcXVpcmVkIGR1cmluZyBwb2xsaW5n IChzdWNoIGFzIGluIGEgLT5kZXRlY3QgaG9vayksIHRoZSBHUFUgd2lsbAo+IGJlIHJ1bnRpbWUg cmVzdW1lZCBldmVyeSAxMCBzZWNzLiAgWW91IG1heSB3YW50IHRvIHZlcmlmeSB0aGF0J3Mgbm90 IHRoZQo+IGNhc2UuICBJZiB5b3UgZGVjaWRlIHRoYXQgeW91IGRvIHdhbnQgdG8gc3RvcCBwb2xs aW5nIGR1cmluZyBydW50aW1lCj4gc3VzcGVuZCBiZWNhdXNlIGl0IHJ1bnRpbWUgcmVzdW1lcyB0 aGUgR1BVIGNvbnRpbnVvdXNseSwgeW91J2xsIG5lZWQgdGhlCj4gaGVscGVyIGludHJvZHVjZWQg aW4gdGhpcyBzZXJpZXMuICBTbyBieSBjYydpbmcgeW91IEkganVzdCB3YW50ZWQgdG8ga2VlcAo+ IHlvdSBpbiB0aGUgbG9vcCBhYm91dCBhbiBpc3N1ZSB0aGF0IG1heSBwb3RlbnRpYWxseSBhZmZl Y3QgeW91ciBkcml2ZXIuCgpBZ2Fpbiwgd2UgaGF2ZSBubyBHUFUgbGlua2VkIHRvIHVzIGFuZCB0 aGUgcG9sbGluZyBkdXJpbmcgcnVudGltZQpzdXNwZW5kIHNob3VsZCBiZSBoYW5kbGVkIGJ5IHRo ZSBkcml2ZXIgZm9yIHRoZSBwYWlyZWQgZW5jb2Rlciwgbm90IGJ5CnVzLiBJIHVuZGVyc3RhbmQg dGhlIHBvdGVudGlhbCBpc3N1ZSBidXQgSSdtIHN0cnVnZ2xpbmcgdG8gdW5kZXJzdGFuZCBpZgpp dCByZWFsbHkgYXBwbGllcyB0byB0aGUgZHJpdmVycy9ncHUvZHJtL2FybSBkcml2ZXJzIG90aGVy IHRoYW4gaW4gYW4KYWJzdHJhY3Qgd2F5LgoKQmVzdCByZWdhcmRzLApMaXZpdQoKPiAKPiBMZXQg bWUga25vdyBpZiB0aGVyZSBhcmUgYW55IHF1ZXN0aW9ucy4KPiAKPiBUaGFua3MsCj4gCj4gTHVr YXMKCi0tIAo9PT09PT09PT09PT09PT09PT09PQp8IEkgd291bGQgbGlrZSB0byB8CnwgZml4IHRo ZSB3b3JsZCwgIHwKfCBidXQgdGhleSdyZSBub3QgfAp8IGdpdmluZyBtZSB0aGUgICB8CiBcIHNv dXJjZSBjb2RlISAgLwogIC0tLS0tLS0tLS0tLS0tLQogICAgwq9cXyjjg4QpXy/CrwpfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGlu ZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK