RFC: Radeon multi ring support branch

Jerome Glisse j.glisse at gmail.com
Fri Nov 18 08:34:42 PST 2011


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


More information about the dri-devel mailing list