RFC: Radeon multi ring support branch

[Jerome Glisse]

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 am not talking about multiple waiter but about single waiter on multilple
signaler. Current implementation allocate one and only one semaphore
not matter how much ring you want to synchronize with. Which means
the single waiter will be unblock once only a single ring signal, which is
broken if you want to wait for several rings.

> 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.

Well the cs patch you posted don't do that. It create one semaphore
and emit a wait with that semaphore and emit signal to all ring. That
was my point. But i agree that creating a semaphore for each ring
couple you want to wait for will work.

>> 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.

Please don't touch the debugfs stuff, it's fine the way it is. No need to
change anything there.

> 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 will take a look

Cheers,
Jerome