[Mesa-dev] [PATCH] compiler: add SYSTEM_VALUE_VARYING_COORD

[Rob Clark]

On Mon, Aug 20, 2018 at 7:20 PM Marek Olšák <maraeo at gmail.com> wrote:
>
> On Mon, Aug 20, 2018 at 7:06 PM Rob Clark <robdclark at gmail.com> wrote:
> >
> > On Mon, Aug 20, 2018 at 6:54 PM Bas Nieuwenhuizen
> > <bas at basnieuwenhuizen.nl> wrote:
> > >
> > > On Tue, Aug 21, 2018 at 12:38 AM, Marek Olšák <maraeo at gmail.com> wrote:
> > > > On Fri, Aug 10, 2018 at 9:26 AM Rob Clark <robdclark at gmail.com> wrote:
> > > >>
> > > >> Used internally in freedreno/ir3 for the vec2 value that hw passes to
> > > >> shader to use as coordinate for bary.f (varying fetch) instruction.
> > > >> This is not the same as SYSTEM_VALUE_FRAG_COORD.
> > > >>
> > > >> Signed-off-by: Rob Clark <robdclark at gmail.com>
> > > >> ---
> > > >> Up until now, we'd been hard-coding the location of this value (ie. to
> > > >> r0.xy), mostly because originally in the early a3xx days I didn't know
> > > >> which bits could configure this value (blob was always using r0.xy so
> > > >> in cmdstream traces it always showed up as 0's).
> > > >>
> > > >> But starting with a6xx, the address register aliases r0.x, which kinda
> > > >> throws a monkey-wrench in the existing scheme of hard-coding.  The good
> > > >> news is that I know the bits to configure this value for a3xx-a6xx.
> > > >>
> > > >> So I'm shifting over to handling this like a sysval.
> > > >>
> > > >>  src/compiler/shader_enums.c                | 1 +
> > > >>  src/compiler/shader_enums.h                | 6 ++++++
> > > >>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
> > > >>  3 files changed, 8 insertions(+)
> > > >>
> > > >> diff --git a/src/compiler/shader_enums.c b/src/compiler/shader_enums.c
> > > >> index a874083a0b7..0210b503d3f 100644
> > > >> --- a/src/compiler/shader_enums.c
> > > >> +++ b/src/compiler/shader_enums.c
> > > >> @@ -244,6 +244,7 @@ gl_system_value_name(gl_system_value sysval)
> > > >>       ENUM(SYSTEM_VALUE_DEVICE_INDEX),
> > > >>       ENUM(SYSTEM_VALUE_VIEW_INDEX),
> > > >>       ENUM(SYSTEM_VALUE_VERTEX_CNT),
> > > >> +     ENUM(SYSTEM_VALUE_VARYING_COORD),
> > > >>     };
> > > >>     STATIC_ASSERT(ARRAY_SIZE(names) == SYSTEM_VALUE_MAX);
> > > >>     return NAME(sysval);
> > > >> diff --git a/src/compiler/shader_enums.h b/src/compiler/shader_enums.h
> > > >> index f8e22925f35..5c36f55283c 100644
> > > >> --- a/src/compiler/shader_enums.h
> > > >> +++ b/src/compiler/shader_enums.h
> > > >> @@ -601,6 +601,12 @@ typedef enum
> > > >>      */
> > > >>     SYSTEM_VALUE_VERTEX_CNT,
> > > >>
> > > >> +   /**
> > > >> +    * Driver internal varying-coord, used for varying-fetch instructions.
> > > >> +    * Not externally visible.
> > > >> +    */
> > > >
> > > > Can you improve the documentation, so that mere mortals understand
> > > > what it means? Does it correspond to something in AMD hw?
> > >
> > > I'd expect this to correspond to stuff like PERSP_CENTER on AMD?
> > >
> > > Which begs the question, does this need distinguishing on
> > > center/centroid/per-sample?
> >
> > so, adreno, being gles hw, has just "smooth" and "flat" varyings..
> >
> > that said, I've been kinda thinking of this as an opaque driver
> > specific sysval without paying too much attention to what the value
> > actually is other than "the thing you pass to bary.f to get non-flat
> > varyings"..  I confess to not having looked into how this works on
> > other hw..
>
> GIven what you said, the variable contains barycentric coordinates
> (i,j) for perspective interpolation at the pixel center. It's the same
> as "vec2 gl_BaryCoordSmoothAMD;" from
> GL_AMD_shader_explicit_vertex_parameter.
>

That seems like a reasonable assumption.. I suppose if there is
mesa+piglit support for gl_BaryCoordSmoothAMD I could try wiring it up
to prove that theory..

BR,
-R