From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757318AbaGWHmL (ORCPT ); Wed, 23 Jul 2014 03:42:11 -0400 Received: from mail-bn1lp0140.outbound.protection.outlook.com ([207.46.163.140]:3538 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757256AbaGWHmI convert rfc822-to-8bit (ORCPT ); Wed, 23 Jul 2014 03:42:08 -0400 X-WSS-ID: 0N95MQ1-08-XH3-02 X-M-MSG: Message-ID: <53CF6746.7010905@amd.com> Date: Wed, 23 Jul 2014 09:41:58 +0200 From: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Maarten Lankhorst , Daniel Vetter CC: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= , "Dave Airlie" , Thomas Hellstrom , nouveau , LKML , dri-devel , Ben Skeggs , "Deucher, Alexander" Subject: Re: [Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences References: <20140709093124.11354.3774.stgit@patser> <20140709122953.11354.46381.stgit@patser> <53CE2421.5040906@amd.com> <20140722114607.GL15237@phenom.ffwll.local> <20140722115737.GN15237@phenom.ffwll.local> <53CE56ED.4040109@vodafone.de> <20140722132652.GO15237@phenom.ffwll.local> <53CE6AFA.1060807@vodafone.de> <53CE84AA.9030703@amd.com> <53CE8A57.2000803@vodafone.de> <53CF58FB.8070609@canonical.com> <53CF5B9F.1050800@amd.com> <53CF5EFE.6070307@canonical.com> <53CF6128.6010703@amd.com> <53CF64F9.7020701@canonical.com> In-Reply-To: <53CF64F9.7020701@canonical.com> Content-Type: text/plain; charset="UTF-8"; format=flowed X-Originating-IP: [10.224.155.198] Content-Transfer-Encoding: 8BIT X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(6009001)(428002)(377454003)(189002)(199002)(24454002)(51704005)(377424004)(107046002)(99396002)(101416001)(85202003)(20776003)(74662001)(106466001)(44976005)(81542001)(92726001)(80316001)(19580405001)(81342001)(95666004)(46102001)(23676002)(47776003)(86362001)(83322001)(59896001)(65806001)(79102001)(87936001)(4396001)(93886003)(97736001)(85852003)(65956001)(19580395003)(80022001)(50466002)(50986999)(77982001)(21056001)(64706001)(76482001)(84676001)(85306003)(85182001)(36756003)(64126003)(68736004)(105586002)(65816999)(33656002)(561944003)(87266999)(102836001)(83506001)(54356999)(76176999)(83072002);DIR:OUT;SFP:;SCL:1;SRVR:BN1PR02MB039;H:atltwp02.amd.com;FPR:;MLV:sfv;PTR:InfoDomainNonexistent;MX:1;LANG:en; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID: X-Forefront-PRVS: 028166BF91 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=Christian.Koenig@amd.com; X-OriginatorOrg: amd4.onmicrosoft.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 23.07.2014 09:32, schrieb Maarten Lankhorst: > op 23-07-14 09:15, Christian König schreef: >> Am 23.07.2014 09:09, schrieb Daniel Vetter: >>> On Wed, Jul 23, 2014 at 9:06 AM, Maarten Lankhorst >>> wrote: >>>>> Can we somehow avoid the need to call fence_signal() at all? The interrupts at least on radeon are way to unreliable for such a thing. Can enable_signalling fail? What's the reason for fence_signaled() in the first place? >>>> It doesn't need to be completely reliable, or finish immediately. >>>> >>>> And any time wake_up_all(&rdev->fence_queue) is called all the fences that were enabled will be rechecked. >>> I raised this already somewhere else, but should we have some common >>> infrastructure in the core fence code to recheck fences periodically? >>> radeon doesn't seem to be the only hw where this isn't reliable >>> enough. Of course timer-based rechecking would only work if the driver >>> provides the fence->signalled callback to recheck actual fence state. >> Yeah, agree. The proposal won't work reliable at all with radeon. >> >> Interrupts are accumulated before sending them to the CPU, e.g. you can get one interrupt for multiple fences finished. If it's just the interrupt for the last fence submitted that gets lost you are completely screwed up because you won't get another interrupt. >> >> I had that problem multiple times while working on UVD support, resulting in the driver thinking that it can't submit more jobs because non of the interrupts for the already submitted fence cam through. > Yeah but all the fences that have .enable_signaling will get signaled from a single interrupt, or when any waiter calls radeon_fence_process. You still need to check if the fence is really signaled, cause radeon_fence_process might wakeup the wait queue because of something completely different. Apart from that you once again rely on somebody else calling radeon_fence_process. This will probably work most of the times, but it's not 100% reliable. > >> Apart from that interrupts on Macs usually don't work at all, so we really need a solution where calling fence_signaled() is completely optional. > I haven't had a problem with interrupts on my mbp after d1f9809ed1315c4cdc5760cf2f59626fd3276952, but it should be trivial to start a timer that periodically does wake_up_all, and gets its timeout reset in a call to radeon_fence_process. It could either be added as a work item, or as a normal timer (disabled during gpu lockup recovery to prevent checks from fiddling with things it shouldn't). That will probably work, but just again sounds like we force the driver to fit the fence implementation instead of the other way around. Why is that fence_signaled() call needed for in the first place? Christian. > > ~Maarten > From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= Subject: Re: [PATCH 09/17] drm/radeon: use common fence implementation for fences Date: Wed, 23 Jul 2014 09:41:58 +0200 Message-ID: <53CF6746.7010905@amd.com> References: <20140709093124.11354.3774.stgit@patser> <20140709122953.11354.46381.stgit@patser> <53CE2421.5040906@amd.com> <20140722114607.GL15237@phenom.ffwll.local> <20140722115737.GN15237@phenom.ffwll.local> <53CE56ED.4040109@vodafone.de> <20140722132652.GO15237@phenom.ffwll.local> <53CE6AFA.1060807@vodafone.de> <53CE84AA.9030703@amd.com> <53CE8A57.2000803@vodafone.de> <53CF58FB.8070609@canonical.com> <53CF5B9F.1050800@amd.com> <53CF5EFE.6070307@canonical.com> <53CF6128.6010703@amd.com> <53CF64F9.7020701@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <53CF64F9.7020701-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: nouveau-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Nouveau" To: Maarten Lankhorst , Daniel Vetter Cc: Thomas Hellstrom , nouveau , LKML , dri-devel , =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= , Ben Skeggs , "Deucher, Alexander" List-Id: nouveau.vger.kernel.org QW0gMjMuMDcuMjAxNCAwOTozMiwgc2NocmllYiBNYWFydGVuIExhbmtob3JzdDoKPiBvcCAyMy0w Ny0xNCAwOToxNSwgQ2hyaXN0aWFuIEvDtm5pZyBzY2hyZWVmOgo+PiBBbSAyMy4wNy4yMDE0IDA5 OjA5LCBzY2hyaWViIERhbmllbCBWZXR0ZXI6Cj4+PiBPbiBXZWQsIEp1bCAyMywgMjAxNCBhdCA5 OjA2IEFNLCBNYWFydGVuIExhbmtob3JzdAo+Pj4gPG1hYXJ0ZW4ubGFua2hvcnN0QGNhbm9uaWNh bC5jb20+IHdyb3RlOgo+Pj4+PiBDYW4gd2Ugc29tZWhvdyBhdm9pZCB0aGUgbmVlZCB0byBjYWxs IGZlbmNlX3NpZ25hbCgpIGF0IGFsbD8gVGhlIGludGVycnVwdHMgYXQgbGVhc3Qgb24gcmFkZW9u IGFyZSB3YXkgdG8gdW5yZWxpYWJsZSBmb3Igc3VjaCBhIHRoaW5nLiBDYW4gZW5hYmxlX3NpZ25h bGxpbmcgZmFpbD8gV2hhdCdzIHRoZSByZWFzb24gZm9yIGZlbmNlX3NpZ25hbGVkKCkgaW4gdGhl IGZpcnN0IHBsYWNlPwo+Pj4+IEl0IGRvZXNuJ3QgbmVlZCB0byBiZSBjb21wbGV0ZWx5IHJlbGlh YmxlLCBvciBmaW5pc2ggaW1tZWRpYXRlbHkuCj4+Pj4KPj4+PiBBbmQgYW55IHRpbWUgd2FrZV91 cF9hbGwoJnJkZXYtPmZlbmNlX3F1ZXVlKSBpcyBjYWxsZWQgYWxsIHRoZSBmZW5jZXMgdGhhdCB3 ZXJlIGVuYWJsZWQgd2lsbCBiZSByZWNoZWNrZWQuCj4+PiBJIHJhaXNlZCB0aGlzIGFscmVhZHkg c29tZXdoZXJlIGVsc2UsIGJ1dCBzaG91bGQgd2UgaGF2ZSBzb21lIGNvbW1vbgo+Pj4gaW5mcmFz dHJ1Y3R1cmUgaW4gdGhlIGNvcmUgZmVuY2UgY29kZSB0byByZWNoZWNrIGZlbmNlcyBwZXJpb2Rp Y2FsbHk/Cj4+PiByYWRlb24gZG9lc24ndCBzZWVtIHRvIGJlIHRoZSBvbmx5IGh3IHdoZXJlIHRo aXMgaXNuJ3QgcmVsaWFibGUKPj4+IGVub3VnaC4gT2YgY291cnNlIHRpbWVyLWJhc2VkIHJlY2hl Y2tpbmcgd291bGQgb25seSB3b3JrIGlmIHRoZSBkcml2ZXIKPj4+IHByb3ZpZGVzIHRoZSBmZW5j ZS0+c2lnbmFsbGVkIGNhbGxiYWNrIHRvIHJlY2hlY2sgYWN0dWFsIGZlbmNlIHN0YXRlLgo+PiBZ ZWFoLCBhZ3JlZS4gVGhlIHByb3Bvc2FsIHdvbid0IHdvcmsgcmVsaWFibGUgYXQgYWxsIHdpdGgg cmFkZW9uLgo+Pgo+PiBJbnRlcnJ1cHRzIGFyZSBhY2N1bXVsYXRlZCBiZWZvcmUgc2VuZGluZyB0 aGVtIHRvIHRoZSBDUFUsIGUuZy4geW91IGNhbiBnZXQgb25lIGludGVycnVwdCBmb3IgbXVsdGlw bGUgZmVuY2VzIGZpbmlzaGVkLiBJZiBpdCdzIGp1c3QgdGhlIGludGVycnVwdCBmb3IgdGhlIGxh c3QgZmVuY2Ugc3VibWl0dGVkIHRoYXQgZ2V0cyBsb3N0IHlvdSBhcmUgY29tcGxldGVseSBzY3Jl d2VkIHVwIGJlY2F1c2UgeW91IHdvbid0IGdldCBhbm90aGVyIGludGVycnVwdC4KPj4KPj4gSSBo YWQgdGhhdCBwcm9ibGVtIG11bHRpcGxlIHRpbWVzIHdoaWxlIHdvcmtpbmcgb24gVVZEIHN1cHBv cnQsIHJlc3VsdGluZyBpbiB0aGUgZHJpdmVyIHRoaW5raW5nIHRoYXQgaXQgY2FuJ3Qgc3VibWl0 IG1vcmUgam9icyBiZWNhdXNlIG5vbiBvZiB0aGUgaW50ZXJydXB0cyBmb3IgdGhlIGFscmVhZHkg c3VibWl0dGVkIGZlbmNlIGNhbSB0aHJvdWdoLgo+IFllYWggYnV0IGFsbCB0aGUgZmVuY2VzIHRo YXQgaGF2ZSAuZW5hYmxlX3NpZ25hbGluZyB3aWxsIGdldCBzaWduYWxlZCBmcm9tIGEgc2luZ2xl IGludGVycnVwdCwgb3Igd2hlbiBhbnkgd2FpdGVyIGNhbGxzIHJhZGVvbl9mZW5jZV9wcm9jZXNz LgoKWW91IHN0aWxsIG5lZWQgdG8gY2hlY2sgaWYgdGhlIGZlbmNlIGlzIHJlYWxseSBzaWduYWxl ZCwgY2F1c2UgCnJhZGVvbl9mZW5jZV9wcm9jZXNzIG1pZ2h0IHdha2V1cCB0aGUgd2FpdCBxdWV1 ZSBiZWNhdXNlIG9mIHNvbWV0aGluZyAKY29tcGxldGVseSBkaWZmZXJlbnQuCgpBcGFydCBmcm9t IHRoYXQgeW91IG9uY2UgYWdhaW4gcmVseSBvbiBzb21lYm9keSBlbHNlIGNhbGxpbmcgCnJhZGVv bl9mZW5jZV9wcm9jZXNzLiBUaGlzIHdpbGwgcHJvYmFibHkgd29yayBtb3N0IG9mIHRoZSB0aW1l cywgYnV0IAppdCdzIG5vdCAxMDAlIHJlbGlhYmxlLgoKPgo+PiBBcGFydCBmcm9tIHRoYXQgaW50 ZXJydXB0cyBvbiBNYWNzIHVzdWFsbHkgZG9uJ3Qgd29yayBhdCBhbGwsIHNvIHdlIHJlYWxseSBu ZWVkIGEgc29sdXRpb24gd2hlcmUgY2FsbGluZyBmZW5jZV9zaWduYWxlZCgpIGlzIGNvbXBsZXRl bHkgb3B0aW9uYWwuCj4gSSBoYXZlbid0IGhhZCBhIHByb2JsZW0gd2l0aCBpbnRlcnJ1cHRzIG9u IG15IG1icCBhZnRlciBkMWY5ODA5ZWQxMzE1YzRjZGM1NzYwY2YyZjU5NjI2ZmQzMjc2OTUyLCBi dXQgaXQgc2hvdWxkIGJlIHRyaXZpYWwgdG8gc3RhcnQgYSB0aW1lciB0aGF0IHBlcmlvZGljYWxs eSBkb2VzIHdha2VfdXBfYWxsLCBhbmQgZ2V0cyBpdHMgdGltZW91dCByZXNldCBpbiBhIGNhbGwg dG8gcmFkZW9uX2ZlbmNlX3Byb2Nlc3MuIEl0IGNvdWxkIGVpdGhlciBiZSBhZGRlZCBhcyBhIHdv cmsgaXRlbSwgb3IgYXMgYSBub3JtYWwgdGltZXIgKGRpc2FibGVkIGR1cmluZyBncHUgbG9ja3Vw IHJlY292ZXJ5IHRvIHByZXZlbnQgY2hlY2tzIGZyb20gZmlkZGxpbmcgd2l0aCB0aGluZ3MgaXQg c2hvdWxkbid0KS4KClRoYXQgd2lsbCBwcm9iYWJseSB3b3JrLCBidXQganVzdCBhZ2FpbiBzb3Vu ZHMgbGlrZSB3ZSBmb3JjZSB0aGUgZHJpdmVyIAp0byBmaXQgdGhlIGZlbmNlIGltcGxlbWVudGF0 aW9uIGluc3RlYWQgb2YgdGhlIG90aGVyIHdheSBhcm91bmQuCgpXaHkgaXMgdGhhdCBmZW5jZV9z aWduYWxlZCgpIGNhbGwgbmVlZGVkIGZvciBpbiB0aGUgZmlyc3QgcGxhY2U/CgpDaHJpc3RpYW4u Cgo+Cj4gfk1hYXJ0ZW4KPgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KTm91dmVhdSBtYWlsaW5nIGxpc3QKTm91dmVhdUBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL25vdXZlYXUK