RFC: Radeon multi ring support branch

Jerome Glisse j.glisse at gmail.com
Sun Nov 20 13:04:34 PST 2011


On Fri, Nov 18, 2011 at 11:34 AM, Jerome Glisse <j.glisse at gmail.com> wrote:
> On Fri, Nov 18, 2011 at 07:44:19AM -0500, Alex Deucher wrote:
>> 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
>>
>
> Ok so reviewed the patch serie, please Christian keep v2, v3, ...
> informations, i find this usefull. I put updated patch at
> http://people.freedesktop.org/~glisse/mrings/
>
> Couple of fixes there, indentation, and also i changed the testing
> parameter to be a bit flag which make our life easier when we want
> to only test the semaphore stuff and not the bo move.
>
> Cheers,
> Jerome
>

Found a major bug introduced by patch 15, rewrote it. Reuploaded
the whole series at
http://people.freedesktop.org/~glisse/mrings/

Issue is that all the fence list should be initialized only once from
asic init callback. Commit message has bigger explanation.

Cheers,
Jerome


More information about the dri-devel mailing list