On Sun, May 24, 2015 at 10:56 AM, Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de> wrote: > > > On 24.05.2015 16:15, Pierre Moreau wrote: >>> >>> On 24 May 2015, at 16:03, Tobias Klausmann >>> <tobias.johannes.klausm...@mni.thm.de> wrote: >>> >>> >>> >>> On 24.05.2015 10:38, Samuel Pitoiset wrote: >>>> >>>> >>>> On 05/24/2015 06:58 AM, Ilia Mirkin wrote: >>>>> >>>>> nv30_validate_clip depends on the rasterizer state. Also we should >>>>> upload all the new clip planes on change since next time the plane data >>>>> won't have changed, but the enables might. >>>>> >>>>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> >>>>> --- >>>>> src/gallium/drivers/nouveau/nv30/nv30_state_validate.c | 16 >>>>> +++++++--------- >>>>> 1 file changed, 7 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >>>>> b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >>>>> index 86ac4f7..a954dcc 100644 >>>>> --- a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >>>>> +++ b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c >>>>> @@ -272,15 +272,13 @@ nv30_validate_clip(struct nv30_context *nv30) >>>>> uint32_t clpd_enable = 0; >>>>> for (i = 0; i < 6; i++) { >>>>> - if (nv30->rast->pipe.clip_plane_enable & (1 << i)) { >>>>> - if (nv30->dirty & NV30_NEW_CLIP) { >>>>> - BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5); >>>>> - PUSH_DATA (push, i); >>>>> - PUSH_DATAp(push, nv30->clip.ucp[i], 4); >>>>> - } >>>>> - >>>>> - clpd_enable |= 1 << (1 + 4*i); >>>>> + if (nv30->dirty & NV30_NEW_CLIP) { >>>>> + BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5); >>>>> + PUSH_DATA (push, i); >>>>> + PUSH_DATAp(push, nv30->clip.ucp[i], 4); >>>>> } >>>>> + if (nv30->rast->pipe.clip_plane_enable & (1 << i)) >>>>> + clpd_enable |= 2 << (4*i); >>>> >>>> Can you explain why did you change this line? >>> >>> This does bother me as well :) >> >> It should be the same as before but using one less addition: shifting 1 by >> 5 or 2 by 4 is similar. > > > *dang* you are right. maybe we should change those lines along in nv50 and > nvc0, save the additional addition :-)
What lines? > > With this sorted out, series is: Not sure what you mean here... what do you want me to sort out? The 2 back into a +1? I was just looking at the defines like #define NV30_3D_VP_CLIP_PLANES_ENABLE_PLANE1 0x00000020 and the 2 << 4i seemed more obviously correct. Although they're obviously identical. > > Reviewed-by: Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de> > > >> >>> >>>>> } >>>>> BEGIN_NV04(push, NV30_3D(VP_CLIP_PLANES_ENABLE), 1); >>>>> @@ -389,7 +387,7 @@ static struct state_validate hwtnl_validate_list[] >>>>> = { >>>>> { nv30_validate_stipple, NV30_NEW_STIPPLE }, >>>>> { nv30_validate_scissor, NV30_NEW_SCISSOR | >>>>> NV30_NEW_RASTERIZER }, >>>>> { nv30_validate_viewport, NV30_NEW_VIEWPORT }, >>>>> - { nv30_validate_clip, NV30_NEW_CLIP }, >>>>> + { nv30_validate_clip, NV30_NEW_CLIP | NV30_NEW_RASTERIZER >>>>> }, >>>>> { nv30_fragprog_validate, NV30_NEW_FRAGPROG | >>>>> NV30_NEW_FRAGCONST }, >>>>> { nv30_vertprog_validate, NV30_NEW_VERTPROG | >>>>> NV30_NEW_VERTCONST | >>>>> NV30_NEW_FRAGPROG | >>>>> NV30_NEW_RASTERIZER }, >>>> >>>> _______________________________________________ >>>> Nouveau mailing list >>>> Nouveau@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/nouveau >>> >>> _______________________________________________ >>> Nouveau mailing list >>> Nouveau@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/nouveau > > _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau