Re: [Mesa-dev] [PATCH] compiler: add SYSTEM_VALUE_VARYING_COORD
On Mon, Aug 20, 2018 at 7:20 PM Marek Olšák wrote: > > On Mon, Aug 20, 2018 at 7:06 PM Rob Clark wrote: > > > > On Mon, Aug 20, 2018 at 6:54 PM Bas Nieuwenhuizen > > wrote: > > > > > > On Tue, Aug 21, 2018 at 12:38 AM, Marek Olšák wrote: > > > > On Fri, Aug 10, 2018 at 9:26 AM Rob Clark 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 > > > >> --- > > > >> 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] compiler: add SYSTEM_VALUE_VARYING_COORD
On Mon, Aug 20, 2018 at 7:06 PM Rob Clark wrote: > > On Mon, Aug 20, 2018 at 6:54 PM Bas Nieuwenhuizen > wrote: > > > > On Tue, Aug 21, 2018 at 12:38 AM, Marek Olšák wrote: > > > On Fri, Aug 10, 2018 at 9:26 AM Rob Clark 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 > > >> --- > > >> 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. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] compiler: add SYSTEM_VALUE_VARYING_COORD
On Mon, Aug 20, 2018 at 6:54 PM Bas Nieuwenhuizen wrote: > > On Tue, Aug 21, 2018 at 12:38 AM, Marek Olšák wrote: > > On Fri, Aug 10, 2018 at 9:26 AM Rob Clark 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 > >> --- > >> 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.. BR, -R > > - Bas > > > > Marek > > ___ > > 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] compiler: add SYSTEM_VALUE_VARYING_COORD
On Tue, Aug 21, 2018 at 12:38 AM, Marek Olšák wrote: > On Fri, Aug 10, 2018 at 9:26 AM Rob Clark 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 >> --- >> 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? - Bas > > Marek > ___ > 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] compiler: add SYSTEM_VALUE_VARYING_COORD
On Fri, Aug 10, 2018 at 9:26 AM Rob Clark 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 > --- > 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? Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] compiler: add SYSTEM_VALUE_VARYING_COORD
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 --- 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. +*/ + SYSTEM_VALUE_VARYING_COORD, + SYSTEM_VALUE_MAX /**< Number of values */ } gl_system_value; diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index aec53309172..ec79a7f4918 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -5601,6 +5601,7 @@ _mesa_sysval_to_semantic(unsigned sysval) case SYSTEM_VALUE_LOCAL_INVOCATION_INDEX: case SYSTEM_VALUE_GLOBAL_INVOCATION_ID: case SYSTEM_VALUE_VERTEX_CNT: + case SYSTEM_VALUE_VARYING_COORD: default: assert(!"Unexpected SYSTEM_VALUE_ enum"); return TGSI_SEMANTIC_COUNT; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
