thr3ads.net - Nouveau - [Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes [May 2015]

If this information is useful, please help other people find it:
Share via:

Samuel Pitoiset

2015-May-24 08:38 UTC

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

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 <imirkin at 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?
>      }
>   
>      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 },

Tobias Klausmann

2015-May-24 14:03 UTC

head link

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

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 <imirkin at 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 :)

>
>>      }
>>        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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Pierre Moreau

2015-May-24 14:15 UTC

head link

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

> On 24 May 2015, at 16:03, Tobias Klausmann <tobias.johannes.klausmann at
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 <imirkin at 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.
> 
> 
>> 
>>>     }
>>>       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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

Possibly Parallel Threads

Search for more seemingly similar threads

Nouveau - May 2015 - [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

Possibly Parallel Threads