[Nouveau] Why do we disable local IRQ around nouveau_fence_update?

[Maarten Maathuis]

On Fri, May 28, 2010 at 7:47 AM, Ben Skeggs <skeggsb at gmail.com> wrote:
> On Thu, 2010-05-27 at 17:55 +0300, Pekka Paalanen wrote:
>> On Wed, 26 May 2010 23:24:57 +0200
>> Maarten Maathuis <madman2003 at gmail.com> wrote:
>>
>> > For NV04 i can understand, since it's irq driven fences, so let's
>> > split the question.
>> >
>> > NV10+: can we reduce it to just spin_lock?
>>
>> I don't know the answer, but I know the theory: if there is
>> any path, that can take the spinlock from an interrupt
>> service path, then you must use the irq-safe version everywhere.
> We could, the interrupt-based path is currently only used on really old
> chips that don't have REF_CNT.
>
>>
>> > NV04: can't we rely on a normal spin lock and add it as well in
>> > nv04_graph_mthd_set_ref?
>>
>> So if NV04 fences are driven by irqs, and the ISR needs to
>> take the lock, then no, you cannot revert to irq-unsafe spinlocks.
>> I'm not sure how it relates to ISR bottom halves, though.
>>
>> Note, that also irq-unsafe spinlocks disable preemption, which
>> might be enough to disturb audio.
> The spinlock was actually only ever meant to protect the list itself and
> not the sequence counters.
>
> I've attached a patch removing the spinlock use everywhere except when
> we're actually going to touch the pending list, I think
> last_sequence_irq is still safe as the NV04 fence IRQ handler is the
> only writer.
>
> I haven't tested beyond knowing this laptop I'm typing on still works.
>
> Thoughts?
>

Why the extra "chan->fence.last_sequence_irq++"? Isn't it already set
from nv04_graph_mthd_set_ref?

>>
>>
>> my 2c
>>
>
>



-- 
Life spent, a precious moment, in the wink of an eye we live and we die.