Include request for reset-rework branch v3

[Jerome Glisse]

On Wed, May 2, 2012 at 7:25 AM, Christian König <deathsimple at vodafone.de> wrote:
> On 02.05.2012 12:32, Dave Airlie wrote:
>>
>> On Wed, May 2, 2012 at 10:04 AM, Christian König
>> <deathsimple at vodafone.de>  wrote:
>>>
>>> On 02.05.2012 06:04, Jerome Glisse wrote:
>>>>
>>>> On Wed, May 2, 2012 at 12:00 AM,<j.glisse at gmail.com>    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