From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755938AbaGVP7k (ORCPT ); Tue, 22 Jul 2014 11:59:40 -0400 Received: from pegasos-out.vodafone.de ([80.84.1.38]:55994 "EHLO pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753793AbaGVP7i (ORCPT ); Tue, 22 Jul 2014 11:59:38 -0400 X-Spam-Flag: NO X-Spam-Score: -0.045 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 D631572552C X-DKIM: OpenDKIM Filter v2.0.2 smtp-04.vodafone.de 566F6E841E Message-ID: <53CE8A57.2000803@vodafone.de> Date: Tue, 22 Jul 2014 17:59:19 +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: Daniel Vetter , =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5p?= =?UTF-8?B?Zw==?= CC: Dave Airlie , Maarten Lankhorst , 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> In-Reply-To: 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 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()? > Locking > correctness is enforced with some extremely nasty lockdep annotations > + additional debugging infrastructure enabled with > CONFIG_DEBUG_WW_MUTEX_SLOWPATH. We really need to be able to hold > dma-buf ww_mutexes while updating fences or waiting for them. And > obviously for ->wait we need non-atomic context, not just > non-interrupt. Sounds mostly reasonable, but for holding the dma-buf ww_mutex, wouldn't be an RCU be more appropriate here? E.g. aren't we just interested that the current assigned fence at some point is signaled? Something like grab ww_mutexes, grab a reference to the current fence object, release ww_mutex, wait for fence, release reference to the fence object. > 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. > So from the core fence framework I think we already have exactly this, > and we only need to adjust the radeon implementation a bit to make it > less risky and invasive to the radeon driver logic. Agree. Well the biggest problem I see is that exclusive semaphore I need to take when anything calls into the driver. For the fence code I need to move that down into the fence->signaled handler, cause that now can be called from outside the driver. Maarten solved this by telling the driver in the lockup handler (where we grab the write side of the exclusive lock) that all interrupts are already enabled, so that fence->signaled hopefully wouldn't mess with the hardware at all. While this probably works, it just leaves me with a feeling that we are doing something wrong here. Christian. > -Daniel 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: Tue, 22 Jul 2014 17:59:19 +0200 Message-ID: <53CE8A57.2000803@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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter , =?UTF-8?B?Q2hyaXN0aWFuIEvDtm5p?= =?UTF-8?B?Zw==?= Cc: Thomas Hellstrom , nouveau , LKML , dri-devel , "Deucher, Alexander" , Ben Skeggs List-Id: nouveau.vger.kernel.org QW0gMjIuMDcuMjAxNCAxNzo0Miwgc2NocmllYiBEYW5pZWwgVmV0dGVyOgo+IE9uIFR1ZSwgSnVs IDIyLCAyMDE0IGF0IDU6MzUgUE0sIENocmlzdGlhbiBLw7ZuaWcKPiA8Y2hyaXN0aWFuLmtvZW5p Z0BhbWQuY29tPiB3cm90ZToKPj4gRHJpdmVycyBleHBvcnRpbmcgZmVuY2VzIG5lZWQgdG8gcHJv dmlkZSBhIGZlbmNlLT5zaWduYWxlZCBhbmQgYSBmZW5jZS0+d2FpdAo+PiBmdW5jdGlvbiwgZXZl cnl0aGluZyBlbHNlIGxpa2UgZmVuY2UtPmVuYWJsZV9zaWduYWxpbmcgb3IgY2FsbGluZwo+PiBm ZW5jZV9zaWduYWxlZCgpIGZyb20gdGhlIGRyaXZlciBpcyBvcHRpb25hbC4KPj4KPj4gRHJpdmVy cyB3YW50aW5nIHRvIHVzZSBleHBvcnRlZCBmZW5jZXMgZG9uJ3QgY2FsbCBmZW5jZS0+c2lnbmFs ZWQgb3IKPj4gZmVuY2UtPndhaXQgaW4gYXRvbWljIG9yIGludGVycnVwdCBjb250ZXh0LCBhbmQg bm90IHdpdGggaG9sZGluZyBhbnkgZ2xvYmFsCj4+IGxvY2tpbmcgcHJpbWl0aXZlcyAobGlrZSBt bWFwX3NlbSBldGMuLi4pLiBIb2xkaW5nIGxvY2tpbmcgcHJpbWl0aXZlcyBsb2NhbAo+PiB0byB0 aGUgZHJpdmVyIGlzIG9rLCBhcyBsb25nIGFzIHRoZXkgZG9uJ3QgY29uZmxpY3Qgd2l0aCBhbnl0 aGluZyBwb3NzaWJsZQo+PiB1c2VkIGJ5IHRoZWlyIG93biBmZW5jZSBpbXBsZW1lbnRhdGlvbi4K PiBXZWxsIHRoYXQncyBhbG1vc3Qgd2hhdCB3ZSBoYXZlIHJpZ2h0IG5vdyB3aXRoIHRoZSBleGNl cHRpb24gdGhhdAo+IGRyaXZlcnMgYXJlIGFsbG93ZWQgKGFjdHVhbGx5IG11c3QgZm9yIGNvcnJl Y3RuZXNzIHdoZW4gdXBkYXRpbmcKPiBmZW5jZXMpIHRoZSB3d19tdXRleGVzIGZvciBkbWEtYnVm cyAob3Igb3RoZXIgYnVmZmVyIG9iamVjdHMpLgoKSW4gdGhpcyBjYXNlIHNvcnJ5IGZvciBzbyBt dWNoIG5vaXNlLiBJIHJlYWxseSBoYXZlbid0IGxvb2tlZCBpbiBzbyBtdWNoIApkZXRhaWwgaW50 byBhbnl0aGluZyBidXQgTWFhcnRlbidzIFJhZGVvbiBwYXRjaGVzLgoKQnV0IGhvdyBkb2VzIHRo YXQgdGhlbiB3b3JrIHJpZ2h0IG5vdz8gTXkgaW1wcmVzc2lvbiB3YXMgdGhhdCBpdCdzIAptYW5k YXRvcnkgZm9yIGRyaXZlcnMgdG8gY2FsbCBmZW5jZV9zaWduYWxlZCgpPwoKPiBMb2NraW5nCj4g Y29ycmVjdG5lc3MgaXMgZW5mb3JjZWQgd2l0aCBzb21lIGV4dHJlbWVseSBuYXN0eSBsb2NrZGVw IGFubm90YXRpb25zCj4gKyBhZGRpdGlvbmFsIGRlYnVnZ2luZyBpbmZyYXN0cnVjdHVyZSBlbmFi bGVkIHdpdGgKPiBDT05GSUdfREVCVUdfV1dfTVVURVhfU0xPV1BBVEguIFdlIHJlYWxseSBuZWVk IHRvIGJlIGFibGUgdG8gaG9sZAo+IGRtYS1idWYgd3dfbXV0ZXhlcyB3aGlsZSB1cGRhdGluZyBm ZW5jZXMgb3Igd2FpdGluZyBmb3IgdGhlbS4gQW5kCj4gb2J2aW91c2x5IGZvciAtPndhaXQgd2Ug bmVlZCBub24tYXRvbWljIGNvbnRleHQsIG5vdCBqdXN0Cj4gbm9uLWludGVycnVwdC4KClNvdW5k cyBtb3N0bHkgcmVhc29uYWJsZSwgYnV0IGZvciBob2xkaW5nIHRoZSBkbWEtYnVmIHd3X211dGV4 LCB3b3VsZG4ndCAKYmUgYW4gUkNVIGJlIG1vcmUgYXBwcm9wcmlhdGUgaGVyZT8gRS5nLiBhcmVu J3Qgd2UganVzdCBpbnRlcmVzdGVkIHRoYXQgCnRoZSBjdXJyZW50IGFzc2lnbmVkIGZlbmNlIGF0 IHNvbWUgcG9pbnQgaXMgc2lnbmFsZWQ/CgpTb21ldGhpbmcgbGlrZSBncmFiIHd3X211dGV4ZXMs IGdyYWIgYSByZWZlcmVuY2UgdG8gdGhlIGN1cnJlbnQgZmVuY2UgCm9iamVjdCwgcmVsZWFzZSB3 d19tdXRleCwgd2FpdCBmb3IgZmVuY2UsIHJlbGVhc2UgcmVmZXJlbmNlIHRvIHRoZSBmZW5jZSAK b2JqZWN0LgoKCj4gQWdyZWVkIHRoYXQgYW55IHNoYXJlZCBsb2NrcyBhcmUgb3V0IG9mIHRoZSB3 YXkgKGVzcGVjaWFsbHkgc3R1ZmYgbGlrZQo+IGRldi0+c3RydWN0X211dGV4IG9yIG90aGVyIG5v bi1zdHJpY3RseSBkcml2ZXItcHJpdmF0ZSBzdHVmZiwgaTkxNSBpcwo+IHJlYWxseSBiYWQgaGVy ZSBzdGlsbCkuCgpZZWFoIHRoYXQncyBhbHNvIGFuIHBvaW50IEkndmUgd2FudGVkIHRvIG5vdGUg b24gTWFhcnRlbnMgcGF0Y2guIFJhZGVvbiAKZ3JhYnMgdGhlIHJlYWQgc2lkZSBvZiBpdCdzIGV4 Y2x1c2l2ZSBzZW1hcGhvcmUgd2hpbGUgd2FpdGluZyBmb3IgZmVuY2VzIAooYmVjYXVzZSBpdCBh c3N1bWVzIHRoYXQgdGhlIGZlbmNlIGl0IHdhaXRzIGZvciBpcyBhIFJhZGVvbiBmZW5jZSkuCgpB c3N1bWluZyB0aGF0IHdlIG5lZWQgdG8gd2FpdCBpbiBib3RoIGRpcmVjdGlvbnMgd2l0aCBQcmlt ZSAoZS5nLiBJbnRlbCAKZHJpdmVyIG5lZWRzIHRvIHdhaXQgZm9yIFJhZGVvbiB0byBmaW5pc2gg cmVuZGVyaW5nIGFuZCBSYWRlb24gbmVlZHMgdG8gCndhaXQgZm9yIEludGVsIHRvIGZpbmlzaCBk aXNwbGF5aW5nKSwgdGhpcyBtaWdodCBiZWNvbWUgYSBwZXJmZWN0IApleGFtcGxlIG9mIGxvY2tp bmcgaW52ZXJzaW9uLgoKPiBTbyBmcm9tIHRoZSBjb3JlIGZlbmNlIGZyYW1ld29yayBJIHRoaW5r IHdlIGFscmVhZHkgaGF2ZSBleGFjdGx5IHRoaXMsCj4gYW5kIHdlIG9ubHkgbmVlZCB0byBhZGp1 c3QgdGhlIHJhZGVvbiBpbXBsZW1lbnRhdGlvbiBhIGJpdCB0byBtYWtlIGl0Cj4gbGVzcyByaXNr eSBhbmQgaW52YXNpdmUgdG8gdGhlIHJhZGVvbiBkcml2ZXIgbG9naWMuCgpBZ3JlZS4gV2VsbCB0 aGUgYmlnZ2VzdCBwcm9ibGVtIEkgc2VlIGlzIHRoYXQgZXhjbHVzaXZlIHNlbWFwaG9yZSBJIG5l ZWQgCnRvIHRha2Ugd2hlbiBhbnl0aGluZyBjYWxscyBpbnRvIHRoZSBkcml2ZXIuIEZvciB0aGUg ZmVuY2UgY29kZSBJIG5lZWQgCnRvIG1vdmUgdGhhdCBkb3duIGludG8gdGhlIGZlbmNlLT5zaWdu YWxlZCBoYW5kbGVyLCBjYXVzZSB0aGF0IG5vdyBjYW4gCmJlIGNhbGxlZCBmcm9tIG91dHNpZGUg dGhlIGRyaXZlci4KCk1hYXJ0ZW4gc29sdmVkIHRoaXMgYnkgdGVsbGluZyB0aGUgZHJpdmVyIGlu IHRoZSBsb2NrdXAgaGFuZGxlciAod2hlcmUgCndlIGdyYWIgdGhlIHdyaXRlIHNpZGUgb2YgdGhl IGV4Y2x1c2l2ZSBsb2NrKSB0aGF0IGFsbCBpbnRlcnJ1cHRzIGFyZSAKYWxyZWFkeSBlbmFibGVk LCBzbyB0aGF0IGZlbmNlLT5zaWduYWxlZCBob3BlZnVsbHkgd291bGRuJ3QgbWVzcyB3aXRoIAp0 aGUgaGFyZHdhcmUgYXQgYWxsLiBXaGlsZSB0aGlzIHByb2JhYmx5IHdvcmtzLCBpdCBqdXN0IGxl YXZlcyBtZSB3aXRoIGEgCmZlZWxpbmcgdGhhdCB3ZSBhcmUgZG9pbmcgc29tZXRoaW5nIHdyb25n IGhlcmUuCgpDaHJpc3RpYW4uCgo+IC1EYW5pZWwKCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxp c3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4v bGlzdGluZm8vZHJpLWRldmVsCg==