[Mesa-dev] [PATCH 3/4] tnl: Maintain the _WindowMap matrix in TNLcontext v2.

[Mathias Fröhlich]

Hi,

On Friday, August 07, 2015 10:28:10 Ilia Mirkin wrote:
> > One alternative I can see is to call _tnl_InvalidateState without the
> > _NEW_VIEWPORT and
> >
> > _NEW_BUFFERS flag set instead of _tnl_wakeup in the two nouveau early
> > initialization
> >
> > code paths in nouveau_swtnl_t.c and nv04_render.c. That should also bring
> > us back to
> >
> > the original behavior.
> >
> 
> That seems like a bit of a hack. If you think it's OK, I'm just going to
> introduce the DrawBuffer check. Brian, does that seem OK to you? I'm mostly
> unfamiliar with the TnL module, its APIs, and when/how they're supposed to
> be used. But nouveau_vieux calls tnl_wakeup right in its context creation
> pass. nv04/nv05 don't have hwtnl at all, nv10+ have it, but it has various
> limitations, with which we end up using swtnl.

To be honest, I did not expect the _tnl_InvalidateState function to be called
much differently than through _mesa_update_state where it is usually called
through a driver provided UpdateState callback. If you look where
_tnl_InvalidateState is used you can see this pattern broadly used.
The only exception is that it is also called through _tnl_wakeup which sounds
to me like something that is not meant for initialization purpose. Actually,
only nouveau uses this call for initialization stuff. So, to me the question
looks like: what is the right way to initialize nouveau.
Either way, just skipping this part of _tnl_InvalidateState either by a
pointer check or something else that makes that part of _tnl_InvalidateState
happen only for the succeeding calls, should be safe in terms of crashes as
well as correctness. As I read the calling code we should end up calling
_tnl_InvalidateState a second time past we have a bound a DrawBuffer
and before the first draw through nouveau_update_state. So the matrix should be
properly set when it is needed for the first draw.
The only aspect I can add to that is:
The additional pointer check happens on each Driver::UpdateState call. May
be it's not worth to care for that, but if we customize nouveaus startup
procedure we do not introduce any additional if code in state validation.
But, I also won't seriously object against your patch!

Thanks for taking care of the problem!

Greetings

Mathias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150807/f625a0b8/attachment.html>