From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756439AbaGWH1Q (ORCPT ); Wed, 23 Jul 2014 03:27:16 -0400 Received: from pegasos-out.vodafone.de ([80.84.1.38]:52332 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752624AbaGWH1O (ORCPT ); Wed, 23 Jul 2014 03:27:14 -0400 X-Spam-Flag: NO X-Spam-Score: 0.2 Authentication-Results: rohrpostix2.prod.vfnet.de (amavisd-new); dkim=pass header.i=@vodafone.de X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de B00776A1F8F X-DKIM: OpenDKIM Filter v2.0.2 smtp-04.vodafone.de B2DD1E5AC2 Message-ID: <53CF63C2.7070407@vodafone.de> Date: Wed, 23 Jul 2014 09:26: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 , =?UTF-8?B?Q2hyaXM=?= =?UTF-8?B?dGlhbiBLw7ZuaWc=?= , Daniel Vetter CC: 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> In-Reply-To: <53CF5EFE.6070307@canonical.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 23.07.2014 09:06, schrieb Maarten Lankhorst: > op 23-07-14 08:52, Christian König schreef: >> Am 23.07.2014 08:40, schrieb Maarten Lankhorst: >>> op 22-07-14 17:59, Christian König schreef: >>>> Am 22.07.2014 17:42, schrieb Daniel Vetter: >>>>> On Tue, Jul 22, 2014 at 5:35 PM, Christian König >>>>> wrote: >>>>>> Drivers exporting fences need to provide a fence->signaled and a fence->wait >>>>>> function, everything else like fence->enable_signaling or calling >>>>>> fence_signaled() from the driver is optional. >>>>>> >>>>>> Drivers wanting to use exported fences don't call fence->signaled or >>>>>> fence->wait in atomic or interrupt context, and not with holding any global >>>>>> locking primitives (like mmap_sem etc...). Holding locking primitives local >>>>>> to the driver is ok, as long as they don't conflict with anything possible >>>>>> used by their own fence implementation. >>>>> Well that's almost what we have right now with the exception that >>>>> drivers are allowed (actually must for correctness when updating >>>>> fences) the ww_mutexes for dma-bufs (or other buffer objects). >>>> In this case sorry for so much noise. I really haven't looked in so much detail into anything but Maarten's Radeon patches. >>>> >>>> But how does that then work right now? My impression was that it's mandatory for drivers to call fence_signaled()? >>> It's only mandatory to call fence_signal() if the .enable_signaling callback has been called, else you can get away with never calling signaling a fence at all before dropping the last refcount to it. >>> This allows you to keep interrupts disabled when you don't need them. >> 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. > >>>>> Agreed that any shared locks are out of the way (especially stuff like >>>>> dev->struct_mutex or other non-strictly driver-private stuff, i915 is >>>>> really bad here still). >>>> Yeah that's also an point I've wanted to note on Maartens patch. Radeon grabs the read side of it's exclusive semaphore while waiting for fences (because it assumes that the fence it waits for is a Radeon fence). >>>> >>>> Assuming that we need to wait in both directions with Prime (e.g. Intel driver needs to wait for Radeon to finish rendering and Radeon needs to wait for Intel to finish displaying), this might become a perfect example of locking inversion. >>> In the preliminary patches where I can sync radeon with other GPU's I've been very careful in all the places that call into fences, to make sure that radeon wouldn't try to handle lockups for a different (possibly also radeon) card. >> That's actually not such a good idea. >> >> In case of a lockup we need to handle the lockup cause otherwise it could happen that radeon waits for the lockup to be resolved and the lockup handling needs to wait for a fence that's never signaled because of the lockup. > The lockup handling calls radeon_fence_wait, not the generic fence_wait. It doesn't call the exported wait function that takes the exclusive_lock in read mode. > And lockdep should have complained if I screwed that up. :-) You screwed it up and lockdep didn't warn you about it :-P It's not a locking problem I'm talking about here. Radeons lockup handling kicks in when anything calls into the driver from the outside, if you have a fence wait function that's called from the outside but doesn't handle lockups you essentially rely on somebody else calling another radeon function for the lockup to be resolved. Christian. > > ~Maarten > From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5pZw==?= Subject: Re: [Nouveau] [PATCH 09/17] drm/radeon: use common fence implementation for fences Date: Wed, 23 Jul 2014 09:26:58 +0200 Message-ID: <53CF63C2.7070407@vodafone.de> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <53CF5EFE.6070307@canonical.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maarten Lankhorst , =?UTF-8?B?Q2hyaXM=?= =?UTF-8?B?dGlhbiBLw7ZuaWc=?= , Daniel Vetter Cc: Thomas Hellstrom , nouveau , LKML , dri-devel , Ben Skeggs , "Deucher, Alexander" List-Id: nouveau.vger.kernel.org QW0gMjMuMDcuMjAxNCAwOTowNiwgc2NocmllYiBNYWFydGVuIExhbmtob3JzdDoKPiBvcCAyMy0w Ny0xNCAwODo1MiwgQ2hyaXN0aWFuIEvDtm5pZyBzY2hyZWVmOgo+PiBBbSAyMy4wNy4yMDE0IDA4 OjQwLCBzY2hyaWViIE1hYXJ0ZW4gTGFua2hvcnN0Ogo+Pj4gb3AgMjItMDctMTQgMTc6NTksIENo cmlzdGlhbiBLw7ZuaWcgc2NocmVlZjoKPj4+PiBBbSAyMi4wNy4yMDE0IDE3OjQyLCBzY2hyaWVi IERhbmllbCBWZXR0ZXI6Cj4+Pj4+IE9uIFR1ZSwgSnVsIDIyLCAyMDE0IGF0IDU6MzUgUE0sIENo cmlzdGlhbiBLw7ZuaWcKPj4+Pj4gPGNocmlzdGlhbi5rb2VuaWdAYW1kLmNvbT4gd3JvdGU6Cj4+ Pj4+PiBEcml2ZXJzIGV4cG9ydGluZyBmZW5jZXMgbmVlZCB0byBwcm92aWRlIGEgZmVuY2UtPnNp Z25hbGVkIGFuZCBhIGZlbmNlLT53YWl0Cj4+Pj4+PiBmdW5jdGlvbiwgZXZlcnl0aGluZyBlbHNl IGxpa2UgZmVuY2UtPmVuYWJsZV9zaWduYWxpbmcgb3IgY2FsbGluZwo+Pj4+Pj4gZmVuY2Vfc2ln bmFsZWQoKSBmcm9tIHRoZSBkcml2ZXIgaXMgb3B0aW9uYWwuCj4+Pj4+Pgo+Pj4+Pj4gRHJpdmVy cyB3YW50aW5nIHRvIHVzZSBleHBvcnRlZCBmZW5jZXMgZG9uJ3QgY2FsbCBmZW5jZS0+c2lnbmFs ZWQgb3IKPj4+Pj4+IGZlbmNlLT53YWl0IGluIGF0b21pYyBvciBpbnRlcnJ1cHQgY29udGV4dCwg YW5kIG5vdCB3aXRoIGhvbGRpbmcgYW55IGdsb2JhbAo+Pj4+Pj4gbG9ja2luZyBwcmltaXRpdmVz IChsaWtlIG1tYXBfc2VtIGV0Yy4uLikuIEhvbGRpbmcgbG9ja2luZyBwcmltaXRpdmVzIGxvY2Fs Cj4+Pj4+PiB0byB0aGUgZHJpdmVyIGlzIG9rLCBhcyBsb25nIGFzIHRoZXkgZG9uJ3QgY29uZmxp Y3Qgd2l0aCBhbnl0aGluZyBwb3NzaWJsZQo+Pj4+Pj4gdXNlZCBieSB0aGVpciBvd24gZmVuY2Ug aW1wbGVtZW50YXRpb24uCj4+Pj4+IFdlbGwgdGhhdCdzIGFsbW9zdCB3aGF0IHdlIGhhdmUgcmln aHQgbm93IHdpdGggdGhlIGV4Y2VwdGlvbiB0aGF0Cj4+Pj4+IGRyaXZlcnMgYXJlIGFsbG93ZWQg KGFjdHVhbGx5IG11c3QgZm9yIGNvcnJlY3RuZXNzIHdoZW4gdXBkYXRpbmcKPj4+Pj4gZmVuY2Vz KSB0aGUgd3dfbXV0ZXhlcyBmb3IgZG1hLWJ1ZnMgKG9yIG90aGVyIGJ1ZmZlciBvYmplY3RzKS4K Pj4+PiBJbiB0aGlzIGNhc2Ugc29ycnkgZm9yIHNvIG11Y2ggbm9pc2UuIEkgcmVhbGx5IGhhdmVu J3QgbG9va2VkIGluIHNvIG11Y2ggZGV0YWlsIGludG8gYW55dGhpbmcgYnV0IE1hYXJ0ZW4ncyBS YWRlb24gcGF0Y2hlcy4KPj4+Pgo+Pj4+IEJ1dCBob3cgZG9lcyB0aGF0IHRoZW4gd29yayByaWdo dCBub3c/IE15IGltcHJlc3Npb24gd2FzIHRoYXQgaXQncyBtYW5kYXRvcnkgZm9yIGRyaXZlcnMg dG8gY2FsbCBmZW5jZV9zaWduYWxlZCgpPwo+Pj4gSXQncyBvbmx5IG1hbmRhdG9yeSB0byBjYWxs IGZlbmNlX3NpZ25hbCgpIGlmIHRoZSAuZW5hYmxlX3NpZ25hbGluZyBjYWxsYmFjayBoYXMgYmVl biBjYWxsZWQsIGVsc2UgeW91IGNhbiBnZXQgYXdheSB3aXRoIG5ldmVyIGNhbGxpbmcgc2lnbmFs aW5nIGEgZmVuY2UgYXQgYWxsIGJlZm9yZSBkcm9wcGluZyB0aGUgbGFzdCByZWZjb3VudCB0byBp dC4KPj4+IFRoaXMgYWxsb3dzIHlvdSB0byBrZWVwIGludGVycnVwdHMgZGlzYWJsZWQgd2hlbiB5 b3UgZG9uJ3QgbmVlZCB0aGVtLgo+PiBDYW4gd2Ugc29tZWhvdyBhdm9pZCB0aGUgbmVlZCB0byBj YWxsIGZlbmNlX3NpZ25hbCgpIGF0IGFsbD8gVGhlIGludGVycnVwdHMgYXQgbGVhc3Qgb24gcmFk ZW9uIGFyZSB3YXkgdG8gdW5yZWxpYWJsZSBmb3Igc3VjaCBhIHRoaW5nLiBDYW4gZW5hYmxlX3Np Z25hbGxpbmcgZmFpbD8gV2hhdCdzIHRoZSByZWFzb24gZm9yIGZlbmNlX3NpZ25hbGVkKCkgaW4g dGhlIGZpcnN0IHBsYWNlPwo+IEl0IGRvZXNuJ3QgbmVlZCB0byBiZSBjb21wbGV0ZWx5IHJlbGlh YmxlLCBvciBmaW5pc2ggaW1tZWRpYXRlbHkuCj4KPiBBbmQgYW55IHRpbWUgd2FrZV91cF9hbGwo JnJkZXYtPmZlbmNlX3F1ZXVlKSBpcyBjYWxsZWQgYWxsIHRoZSBmZW5jZXMgdGhhdCB3ZXJlIGVu YWJsZWQgd2lsbCBiZSByZWNoZWNrZWQuCj4KPj4+Pj4gQWdyZWVkIHRoYXQgYW55IHNoYXJlZCBs b2NrcyBhcmUgb3V0IG9mIHRoZSB3YXkgKGVzcGVjaWFsbHkgc3R1ZmYgbGlrZQo+Pj4+PiBkZXYt PnN0cnVjdF9tdXRleCBvciBvdGhlciBub24tc3RyaWN0bHkgZHJpdmVyLXByaXZhdGUgc3R1ZmYs IGk5MTUgaXMKPj4+Pj4gcmVhbGx5IGJhZCBoZXJlIHN0aWxsKS4KPj4+PiBZZWFoIHRoYXQncyBh bHNvIGFuIHBvaW50IEkndmUgd2FudGVkIHRvIG5vdGUgb24gTWFhcnRlbnMgcGF0Y2guIFJhZGVv biBncmFicyB0aGUgcmVhZCBzaWRlIG9mIGl0J3MgZXhjbHVzaXZlIHNlbWFwaG9yZSB3aGlsZSB3 YWl0aW5nIGZvciBmZW5jZXMgKGJlY2F1c2UgaXQgYXNzdW1lcyB0aGF0IHRoZSBmZW5jZSBpdCB3 YWl0cyBmb3IgaXMgYSBSYWRlb24gZmVuY2UpLgo+Pj4+Cj4+Pj4gQXNzdW1pbmcgdGhhdCB3ZSBu ZWVkIHRvIHdhaXQgaW4gYm90aCBkaXJlY3Rpb25zIHdpdGggUHJpbWUgKGUuZy4gSW50ZWwgZHJp dmVyIG5lZWRzIHRvIHdhaXQgZm9yIFJhZGVvbiB0byBmaW5pc2ggcmVuZGVyaW5nIGFuZCBSYWRl b24gbmVlZHMgdG8gd2FpdCBmb3IgSW50ZWwgdG8gZmluaXNoIGRpc3BsYXlpbmcpLCB0aGlzIG1p Z2h0IGJlY29tZSBhIHBlcmZlY3QgZXhhbXBsZSBvZiBsb2NraW5nIGludmVyc2lvbi4KPj4+IElu IHRoZSBwcmVsaW1pbmFyeSBwYXRjaGVzIHdoZXJlIEkgY2FuIHN5bmMgcmFkZW9uIHdpdGggb3Ro ZXIgR1BVJ3MgSSd2ZSBiZWVuIHZlcnkgY2FyZWZ1bCBpbiBhbGwgdGhlIHBsYWNlcyB0aGF0IGNh bGwgaW50byBmZW5jZXMsIHRvIG1ha2Ugc3VyZSB0aGF0IHJhZGVvbiB3b3VsZG4ndCB0cnkgdG8g aGFuZGxlIGxvY2t1cHMgZm9yIGEgZGlmZmVyZW50IChwb3NzaWJseSBhbHNvIHJhZGVvbikgY2Fy ZC4KPj4gVGhhdCdzIGFjdHVhbGx5IG5vdCBzdWNoIGEgZ29vZCBpZGVhLgo+Pgo+PiBJbiBjYXNl IG9mIGEgbG9ja3VwIHdlIG5lZWQgdG8gaGFuZGxlIHRoZSBsb2NrdXAgY2F1c2Ugb3RoZXJ3aXNl IGl0IGNvdWxkIGhhcHBlbiB0aGF0IHJhZGVvbiB3YWl0cyBmb3IgdGhlIGxvY2t1cCB0byBiZSBy ZXNvbHZlZCBhbmQgdGhlIGxvY2t1cCBoYW5kbGluZyBuZWVkcyB0byB3YWl0IGZvciBhIGZlbmNl IHRoYXQncyBuZXZlciBzaWduYWxlZCBiZWNhdXNlIG9mIHRoZSBsb2NrdXAuCj4gVGhlIGxvY2t1 cCBoYW5kbGluZyBjYWxscyByYWRlb25fZmVuY2Vfd2FpdCwgbm90IHRoZSBnZW5lcmljIGZlbmNl X3dhaXQuIEl0IGRvZXNuJ3QgY2FsbCB0aGUgZXhwb3J0ZWQgd2FpdCBmdW5jdGlvbiB0aGF0IHRh a2VzIHRoZSBleGNsdXNpdmVfbG9jayBpbiByZWFkIG1vZGUuCj4gQW5kIGxvY2tkZXAgc2hvdWxk IGhhdmUgY29tcGxhaW5lZCBpZiBJIHNjcmV3ZWQgdGhhdCB1cC4gOi0pCgpZb3Ugc2NyZXdlZCBp dCB1cCBhbmQgbG9ja2RlcCBkaWRuJ3Qgd2FybiB5b3UgYWJvdXQgaXQgOi1QCgpJdCdzIG5vdCBh IGxvY2tpbmcgcHJvYmxlbSBJJ20gdGFsa2luZyBhYm91dCBoZXJlLiBSYWRlb25zIGxvY2t1cCAK aGFuZGxpbmcga2lja3MgaW4gd2hlbiBhbnl0aGluZyBjYWxscyBpbnRvIHRoZSBkcml2ZXIgZnJv bSB0aGUgb3V0c2lkZSwgCmlmIHlvdSBoYXZlIGEgZmVuY2Ugd2FpdCBmdW5jdGlvbiB0aGF0J3Mg Y2FsbGVkIGZyb20gdGhlIG91dHNpZGUgYnV0IApkb2Vzbid0IGhhbmRsZSBsb2NrdXBzIHlvdSBl c3NlbnRpYWxseSByZWx5IG9uIHNvbWVib2R5IGVsc2UgY2FsbGluZyAKYW5vdGhlciByYWRlb24g ZnVuY3Rpb24gZm9yIHRoZSBsb2NrdXAgdG8gYmUgcmVzb2x2ZWQuCgpDaHJpc3RpYW4uCgo+Cj4g fk1hYXJ0ZW4KPgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn Cmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK