From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: RFC: Radeon multi ring support branch Date: Thu, 17 Nov 2011 17:24:49 +0100 Message-ID: <4EC53551.8070005@vodafone.de> References: <4EABF8EC.3020701@vodafone.de> <20111115193212.GB2263@homer.localdomain> <4EC2F378.9090801@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 6B1539EB05 for ; Thu, 17 Nov 2011 08:25:02 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Jerome Glisse Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 16.11.2011 01:24, Jerome Glisse wrote: > Well as we don't specify on which value semaphore should wait on, i am > prety sure the first ring to increment the semaphore will unblock all > waiter. So if you have ring1 that want to wait on ring2 and ring3 as > soon as ring2 or ring3 is done ring1 will go one while either ring2 or > ring3 might not be done. I will test that tomorrow but from doc i have > it seems so. Thus it will be broken with more than one ring, that > would mean you have to allocate one semaphore for each ring couple you > want to synchronize. Note that the usual case will likely be sync btw > 2 ring. Good point, but I played with it a bit more today and it is just behaving as I thought it would be. A single signal command will just unblock a single waiter, even if there are multiple waiters currently for this semaphore, the only thing you can't tell is which waiter will come first. I should also note that the current algorithm will just emit multiple wait operations to a single ring, and spread the signal operations to all other rings we are interested in. That isn't very efficient, but should indeed work quite fine. > After retesting the first patch drm/radeon: fix debugfs handling is NAK > a complete no go. > > Issue is that radeon_debugfs_cleanup is call after rdev is free. This > is why i used a static array. I forgot about that, i should have put a > comment. I guess you built your kernel without debugfs or that you > didn't tested to reload the module. Mhm, I have tested it, seen the crash, and didn't thought that this is a problem. Don't ask me why I can't understand it myself right now. Anyway, I moved the unregistering of the files into a separate function, which is now called from radeon_device_fini instead of radeon_debugfs_cleanup. That seems to work fine, at least if I haven't missed something else. I also merged your indention fixes and the fix for the never allocated semaphores and pushed the result into my public repository (http://cgit.freedesktop.org/~deathsimple/linux), so please take another look at it. Thanks, Christian.