On Fri, Nov 18, 2011 at 11:34 AM, Jerome Glisse <j.glisse@xxxxxxxxx> wrote: > On Fri, Nov 18, 2011 at 07:44:19AM -0500, Alex Deucher wrote: >> 2011/11/17 Alex Deucher <alexdeucher@xxxxxxxxx>: >> > 2011/11/17 Christian König <deathsimple@xxxxxxxxxxx>: >> >> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
