On Wed, May 2, 2012 at 7:25 AM, Christian König <deathsimple@xxxxxxxxxxx> wrote: > On 02.05.2012 12:32, Dave Airlie wrote: >> >> On Wed, May 2, 2012 at 10:04 AM, Christian König >> <deathsimple@xxxxxxxxxxx> wrote: >>> >>> On 02.05.2012 06:04, Jerome Glisse wrote: >>>> >>>> On Wed, May 2, 2012 at 12:00 AM,<j.glisse@xxxxxxxxx> wrote: >>>>> >>>>> Ok so i reread stuff and the : >>>>> drm/radeon: add general purpose fence signaled callback >>>>> is a big NAK actually. It change the paradigm. Moving most of >>>>> the handling into the irq process which is something i am intimatly >>>>> convinced we should avoid. >>>>> >>>>> Here is the patchset up to ib pool cleanup. I have yet rebase the >>>>> other patches as i am tracking done some issue in the sa allocation. >>>>> >>>>> Cheers, >>>>> Jerome >>>>> >>>> Before i forget, the big issue with doing work from irq handler is that >>>> we never know in middle of what other part can be. I believe it's lot >>>> better to have irq process only update fence (signaled/not signaled). >>>> and have the actual work happening on behalf of the process wether >>>> through sa alloc path or ttm path. >>> >>> >>> Disagree. >>> >>> Why should it be better to delay work outside of the interrupt context if >>> proper locking can make the driver much more responsive and easier to >>> implement? >>> >>> I don't want to call into TTM or stuff like that, just want make it >>> possible >>> to release the resources acquired for a job immediately after the job is >>> completed instead of waiting for the next allocation to happen. Cause >>> then >>> you don't need to check if a bunch of fences might possible be signaled >>> and >>> instead just get a proper signal that resources can be deallocated. >>> >>> Also if you really want to keep the irq context out of the drivers upper >>> layers, it should be quite easy to modify the code so that the callback >>> won't happen from there. >> >> as a general rule try an minimise how much work we do in irq context, >> the locking gets very messy once you have to use a mutex somewhere >> else in the future. > > Akk, that sounds reasonable, but I still think that a fence should signal > it's completion by itself. Because that releases us from the burden to walk > the list of emitted fences and heuristically check if any of them is already > signaled. > > Also it is pretty easy to move the callback outside of interrupt context, > but first things first. I'm going to write together a patchset with > everything that is already accepted, so we can stop mailing around actually > unrelated patches. > > Thanks for the comments, > Christian. > Yes i agree, the fence should check for itself, irq process should only write a per ring seq_last value (probably good to use an atomic one for this) and when querying fence status the signaling should happen. There is 2 possibilities there, either we keep 32bits seq and keep list. Or we move toward 64bit seq and use arithmetic to know if a fence is signaled or not (assuming that we will never wrap around 64bits fence counter in up time). But i am still against callback it's just make locking a mess. As discussed previously i think we should be able to have at most 4 lock: dispatch lock (all ring all gpu related activities) ttm lock temporary memory alloc lock fence lock (that one can go away if we don't keep a list of fence anymore) idea is that either ttm path or temporary memory path might call in fence checking. Cheers, Jerome Glisse Cheers, Jerome _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
