[Mesa-dev] [PATCH v3 1/2] etnaviv: fix varying interpolation

[Wladimir J. van der Laan]

Hello Lucas,

On Fri, Sep 22, 2017 at 11:27:36AM +0200, Lucas Stach wrote:
> It seems that newer cores don't use the PA_ATTRIBUTES to decide if the
> varying should bypass the flat shading, but derive this from the component
> use. This fixes flat shading on GC880+.
> 
> VARYING_COMPONENT_USE_POINTCOORD is a bit of a misnomer now, as it isn't
> only used for pointcoords, but missing a better name I left it as-is.

I was just looking at recent command streams, and it appears that the blob
uses VARYING_COMPONENT_USE_UNUSED even for active components.
I've not seen it use VARYING_COMPONENT_USE_USED at all anymore.

Maybe that works instead of using the POINTCOORD for this? I'm glad
this solved the issue, and I'm ok with merging this as-is, but I have never
seen the blob use POINTCOORD for non-pointcoords and feel it would potentially
interfere with point sprites in some cases.

Regards,
Wladimir

> 
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> Reviewed-by: Philipp Zabel <p.zabel at pengutronix.de>
> Reviewed-by: Wladimir J. van der Laan <laanwj at gmail.com>
> ---
> v2: fix invalid vreg assignment
> v3: fix missed negation, improve variable naming
> ---
>  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> index 165ab74298a4..0f6a5d23425d 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> @@ -2607,6 +2607,7 @@ etna_link_shader(struct etna_shader_link_info *info,
>        const struct etna_shader_inout *fsio = &fs->infile.reg[idx];
>        const struct etna_shader_inout *vsio = etna_shader_vs_lookup(vs, fsio);
>        struct etna_varying *varying;
> +      bool interpolate_always = fsio->semantic.Name != TGSI_SEMANTIC_COLOR;
>  
>        assert(fsio->reg > 0 && fsio->reg <= ARRAY_SIZE(info->varyings));
>  
> @@ -2616,27 +2617,24 @@ etna_link_shader(struct etna_shader_link_info *info,
>        varying = &info->varyings[fsio->reg - 1];
>        varying->num_components = fsio->num_components;
>  
> -      if (fsio->semantic.Name == TGSI_SEMANTIC_COLOR) /* colors affected by flat shading */
> +      if (!interpolate_always) /* colors affected by flat shading */
>           varying->pa_attributes = 0x200;
>        else /* texture coord or other bypasses flat shading */
>           varying->pa_attributes = 0x2f1;
>  
> -      if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD) {
> -         varying->use[0] = VARYING_COMPONENT_USE_POINTCOORD_X;
> -         varying->use[1] = VARYING_COMPONENT_USE_POINTCOORD_Y;
> -         varying->use[2] = VARYING_COMPONENT_USE_USED;
> -         varying->use[3] = VARYING_COMPONENT_USE_USED;
> -         varying->reg = 0; /* replaced by point coord -- doesn't matter */
> +      varying->use[0] = interpolate_always ? VARYING_COMPONENT_USE_POINTCOORD_X : VARYING_COMPONENT_USE_USED;
> +      varying->use[1] = interpolate_always ? VARYING_COMPONENT_USE_POINTCOORD_Y : VARYING_COMPONENT_USE_USED;
> +      varying->use[2] = VARYING_COMPONENT_USE_USED;
> +      varying->use[3] = VARYING_COMPONENT_USE_USED;
> +
> +
> +      /* point coord is position output from VS, so has no dedicated reg */
> +      if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD)
>           continue;
> -      }
>  
>        if (vsio == NULL)
>           return true; /* not found -- link error */
>  
> -      varying->use[0] = VARYING_COMPONENT_USE_USED;
> -      varying->use[1] = VARYING_COMPONENT_USE_USED;
> -      varying->use[2] = VARYING_COMPONENT_USE_USED;
> -      varying->use[3] = VARYING_COMPONENT_USE_USED;
>        varying->reg = vsio->reg;
>     }
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> etnaviv mailing list
> etnaviv at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv