2011/11/17 Alex Deucher <alexdeucher at gmail.com>:
> 2011/11/17 Christian K?nig <deathsimple at vodafone.de>:
>> 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.
>
> I've got a few other patches to enable further functionality in the
> mring patches.
> - per ring fence interrupts
> - add some additional ring fields to better handle different ring types
>
> http://people.freedesktop.org/~agd5f/mrings/
>

FYI, I updated these later last night.

Alex

> Alex
>

Reply via email to