Nouveau - Jun 2014 - [Mesa-dev] [PATCH try 2 2/2] gallium/nouveau: move pushbuf and fences to context

On Mon, Jun 23, 2014 at 5:17 PM, Maarten Lankhorst
<maarten.lankhorst at canonical.com> wrote:
> op 21-06-14 14:12, Ilia Mirkin schreef:
>> On Tue, Jun 17, 2014 at 2:34 AM, Maarten Lankhorst
>> <maarten.lankhorst at canonical.com> wrote:
>>> nv30 seems to not support dma objects with offset, so simply extend
the query_heap to cover the
>>> entire notifier, and use a offset in nv30_context_kick_notify.
>> It would be great if you could detail the list of transformations that
>> were done in the commit description, as well as what the "new
way" is
>> (if any) for the various concepts.
> I moved the pushbuf and fences to each context separately. The PUSH_KICK on
context switch ensures
> that the previous context is flushed.
>> This change doesn't have any of the locking -- is that coming in a
>> future change? Otherwise we're still vulnerable to multiple threads
>> trying to render at the same time. (Now with screen sharing, even if
>> they end up with separate screens, we'd still be in trouble.)
> I haven't done any locking changes, because I don't believe locking
is the answer here.
> With each context having its own pushbuf we can ensure that things
aren't flushed, so
> on flush it should assume all state is dirty. After this is done the
PUSH_KICK of the old
> context on context switch can be removed, and suddenly the driver is
thread-safe because
> only the pushbuf to kernel command submission could race. ;-)
It would be interesting to see some numbers on the impact of assuming
all state is lost each flush vs doing the locking.
>
>> I'm still a bit concerned with moving the fence stuff to the
>> context... there might be an assumption in gallium that fences are
>> context-independent, in which case you need to make sure to have just
>> a single fence shared by everything...
> I don't think that this is the case, because it's very rare that
gallium uses multiple contexts simultaneously.
> (Except vdpau interop, which does flush explicitly.)
>> Have you done a full piglit run on this (with the glx tests, for good
>> measure) on nv30/nv50/nvc0? If so, can you share the results files
>> somewhere?
> No not yet. But I did confirm that glxgears and glxinfo didn't regress
on my nv43, nv96 and nvc0. :-)
>
> ~Maarten
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

op 23-06-14 09:24, Ben Skeggs schreef:
> On Mon, Jun 23, 2014 at 5:17 PM, Maarten Lankhorst
> <maarten.lankhorst at canonical.com> wrote:
>> op 21-06-14 14:12, Ilia Mirkin schreef:
>>> On Tue, Jun 17, 2014 at 2:34 AM, Maarten Lankhorst
>>> <maarten.lankhorst at canonical.com> wrote:
>>>> nv30 seems to not support dma objects with offset, so simply
extend the query_heap to cover the
>>>> entire notifier, and use a offset in nv30_context_kick_notify.
>>> It would be great if you could detail the list of transformations
that
>>> were done in the commit description, as well as what the "new
way" is
>>> (if any) for the various concepts.
>> I moved the pushbuf and fences to each context separately. The
PUSH_KICK on context switch ensures
>> that the previous context is flushed.
>>> This change doesn't have any of the locking -- is that coming
in a
>>> future change? Otherwise we're still vulnerable to multiple
threads
>>> trying to render at the same time. (Now with screen sharing, even
if
>>> they end up with separate screens, we'd still be in trouble.)
>> I haven't done any locking changes, because I don't believe
locking is the answer here.
>> With each context having its own pushbuf we can ensure that things
aren't flushed, so
>> on flush it should assume all state is dirty. After this is done the
PUSH_KICK of the old
>> context on context switch can be removed, and suddenly the driver is
thread-safe because
>> only the pushbuf to kernel command submission could race. ;-)
> It would be interesting to see some numbers on the impact of assuming
> all state is lost each flush vs doing the locking.
Using locking would mean only a single thread could do command submission at a
time, so it still wouldn't be true multithreaded opengl.
And there still seem to be some cases in which this isn't enough, for
example the query stuff should probably become a dirty flag for validation.

~Maarten

On Mon, Jun 23, 2014 at 5:39 PM, Maarten Lankhorst
<maarten.lankhorst at canonical.com> wrote:
> op 23-06-14 09:24, Ben Skeggs schreef:
>> On Mon, Jun 23, 2014 at 5:17 PM, Maarten Lankhorst
>> <maarten.lankhorst at canonical.com> wrote:
>>> op 21-06-14 14:12, Ilia Mirkin schreef:
>>>> On Tue, Jun 17, 2014 at 2:34 AM, Maarten Lankhorst
>>>> <maarten.lankhorst at canonical.com> wrote:
>>>>> nv30 seems to not support dma objects with offset, so
simply extend the query_heap to cover the
>>>>> entire notifier, and use a offset in
nv30_context_kick_notify.
>>>> It would be great if you could detail the list of
transformations that
>>>> were done in the commit description, as well as what the
"new way" is
>>>> (if any) for the various concepts.
>>> I moved the pushbuf and fences to each context separately. The
PUSH_KICK on context switch ensures
>>> that the previous context is flushed.
>>>> This change doesn't have any of the locking -- is that
coming in a
>>>> future change? Otherwise we're still vulnerable to multiple
threads
>>>> trying to render at the same time. (Now with screen sharing,
even if
>>>> they end up with separate screens, we'd still be in
trouble.)
>>> I haven't done any locking changes, because I don't believe
locking is the answer here.
>>> With each context having its own pushbuf we can ensure that things
aren't flushed, so
>>> on flush it should assume all state is dirty. After this is done
the PUSH_KICK of the old
>>> context on context switch can be removed, and suddenly the driver
is thread-safe because
>>> only the pushbuf to kernel command submission could race. ;-)
>> It would be interesting to see some numbers on the impact of assuming
>> all state is lost each flush vs doing the locking.
> Using locking would mean only a single thread could do command submission
at a time, so it still wouldn't be true multithreaded opengl.
> And there still seem to be some cases in which this isn't enough, for
example the query stuff should probably become a dirty flag for validation.
Well, other threads are free to do everything else except the final
draw call still.  But yes, that's potentially a good chunk of the
work.  That can possibly be worked around in a couple of ways if the
numbers show it's worth bothering.
>
> ~Maarten