Re: [Mesa-dev] [PATCH 3/5] ffvertex: Don't try to read output registers in fog calculation

[Ian Romanick]

On 05/06/2018 05:31 PM, Timothy Arceri wrote:
> On 05/05/18 07:28, Kenneth Graunke wrote:
> 
>> On Thursday, April 26, 2018 2:28:41 PM PDT Ian Romanick wrote:
>>> From: Ian Romanick 
>>>
>>> This code was clearly never tested.  It's a good thing that no driver
>>> ever tried to enable this path.
>>>
>>> Signed-off-by: Ian Romanick 
>>> ---
>>>   src/mesa/main/ffvertex_prog.c | 10 ++
>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>> That's not quite accurate.  Gallium drivers advertise NV_fog_distance,
>> but st_program calls _mesa_remove_output_reads(), which presumably
>> lowers those away.  It looks like none of the classic drivers are
>> calling that, though, so they would be broken if they tried to enable
>> this.  Either your patch, or calling _mesa_remove_output_reads(),
>> would probably solve the issue.
>>
>> At any rate, using a temporary seems reasonable.
> 
> Since this patch landed. Do you guys think we can now remove
> 
> _mesa_remove_output_reads() entirely?

Was this bit of ffvertex the only reason that function existed?  Is it
still needed for ARB assembly vertex programs?

> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] ffvertex: Don't try to read output registers in fog calculation

[Timothy Arceri]

On 05/05/18 07:28, Kenneth Graunke wrote:


On Thursday, April 26, 2018 2:28:41 PM PDT Ian Romanick wrote:

From: Ian Romanick 

This code was clearly never tested.  It's a good thing that no driver
ever tried to enable this path.

Signed-off-by: Ian Romanick 
---
  src/mesa/main/ffvertex_prog.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

That's not quite accurate.  Gallium drivers advertise NV_fog_distance,
but st_program calls _mesa_remove_output_reads(), which presumably
lowers those away.  It looks like none of the classic drivers are
calling that, though, so they would be broken if they tried to enable
this.  Either your patch, or calling _mesa_remove_output_reads(),
would probably solve the issue.

At any rate, using a temporary seems reasonable.


Since this patch landed. Do you guys think we can now remove

_mesa_remove_output_reads() entirely?

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/5] ffvertex: Don't try to read output registers in fog calculation

[Kenneth Graunke]

On Thursday, April 26, 2018 2:28:41 PM PDT Ian Romanick wrote:
> From: Ian Romanick 
> 
> This code was clearly never tested.  It's a good thing that no driver
> ever tried to enable this path.
> 
> Signed-off-by: Ian Romanick 
> ---
>  src/mesa/main/ffvertex_prog.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

That's not quite accurate.  Gallium drivers advertise NV_fog_distance,
but st_program calls _mesa_remove_output_reads(), which presumably
lowers those away.  It looks like none of the classic drivers are
calling that, though, so they would be broken if they tried to enable
this.  Either your patch, or calling _mesa_remove_output_reads(),
would probably solve the issue.

At any rate, using a temporary seems reasonable.

Series is:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/5] ffvertex: Don't try to read output registers in fog calculation

[Ian Romanick]

From: Ian Romanick 

This code was clearly never tested.  It's a good thing that no driver
ever tried to enable this path.

Signed-off-by: Ian Romanick 
---
 src/mesa/main/ffvertex_prog.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c
index 1c755592bd9..dfb494bd8c1 100644
--- a/src/mesa/main/ffvertex_prog.c
+++ b/src/mesa/main/ffvertex_prog.c
@@ -1297,12 +1297,14 @@ static void build_fog( struct tnl_program *p )
struct ureg input;
 
switch (p->state->fog_distance_mode) {
-   case FDM_EYE_RADIAL: /* Z = sqrt(Xe*Xe + Ye*Ye + Ze*Ze) */
+   case FDM_EYE_RADIAL: { /* Z = sqrt(Xe*Xe + Ye*Ye + Ze*Ze) */
+  struct ureg tmp = get_temp(p);
   input = get_eye_position(p);
-  emit_op2(p, OPCODE_DP3, fog, WRITEMASK_X, input, input);
-  emit_op1(p, OPCODE_RSQ, fog, WRITEMASK_X, fog);
-  emit_op1(p, OPCODE_RCP, fog, WRITEMASK_X, fog);
+  emit_op2(p, OPCODE_DP3, tmp, WRITEMASK_X, input, input);
+  emit_op1(p, OPCODE_RSQ, tmp, WRITEMASK_X, tmp);
+  emit_op1(p, OPCODE_RCP, fog, WRITEMASK_X, tmp);
   break;
+   }
case FDM_EYE_PLANE: /* Z = Ze */
   input = get_eye_position_z(p);
   emit_op1(p, OPCODE_MOV, fog, WRITEMASK_X, input);
-- 
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev