[meta-rockchip][PATCH] glmark2: fix bugs on panfrost

[meta-rockchip][PATCH] glmark2: fix bugs on panfrost

[Trevor Woerner]

Alyssa submitted this set of patches for glmark2 to fix/clarify glmark2's
handling of fp16 overflows in a shader (which succeed if the driver doesn't
handle fp16 and uses fp32 instead).

See: https://github.com/glmark2/glmark2/pull/132

This is added as a dynamic layer since meta-openembedded/meta-oe isn't a
requirement for this layer.

Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 conf/layer.conf                               |   6 +
 .../files/fix-precision-handling-bugs.patch   | 138 ++++++++++++++++++
 .../glmark2/glmark2_%.bbappend                |   5 +
 3 files changed, 149 insertions(+)
 create mode 100644 dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
 create mode 100644 dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend

diff --git a/conf/layer.conf b/conf/layer.conf
index 8eecdc5..e4b3055 100644
--- a/conf/layer.conf
+++ b/conf/layer.conf
@@ -16,3 +16,9 @@ BBFILE_PRIORITY_rockchip = "1"
 LAYERVERSION_rockchip = "1"
 LAYERSERIES_COMPAT_rockchip = "gatesgarth"
 LAYERDEPENDS_rockchip = "core meta-arm"
+
+# dynamic layers
+BBFILES += "${@' '.join('${LAYERDIR}/dynamic-layers/%s/recipes*/*/*.bbappend' % layer \
+               for layer in BBFILE_COLLECTIONS.split())}"
+BBFILES += "${@' '.join('${LAYERDIR}/dynamic-layers/%s/recipes*/*/*.bb' % layer \
+               for layer in BBFILE_COLLECTIONS.split())}"
diff --git a/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
new file mode 100644
index 0000000..af88f6c
--- /dev/null
+++ b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
@@ -0,0 +1,138 @@
+From 90e837ffd1ff5c9add1074d69de23e58a3a4810e Mon Sep 17 00:00:00 2001
+From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+Date: Wed, 11 Nov 2020 09:26:03 -0500
+Subject: [PATCH 1/3] terrain: Fix precision bug in light rendering
+
+Resulting in overly bright rendering when mediump is implemented as
+fp16.
+
+Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+---
+ data/shaders/terrain.frag | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/data/shaders/terrain.frag b/data/shaders/terrain.frag
+index 84d085c..58f17ea 100644
+--- a/data/shaders/terrain.frag
++++ b/data/shaders/terrain.frag
+@@ -67,7 +67,12 @@ void main() {
+     vec3 pointSpecular = vec3( 0.0 );
+     for ( int i = 0; i < MAX_POINT_LIGHTS; i ++ ) {
+         vec4 lPosition = viewMatrix * vec4( pointLightPosition[ i ], 1.0 );
++#ifdef GL_FRAGMENT_PRECISION_HIGH
++        // should be highp for correct behaviour if mediump is implemented as fp16
++        highp vec3 lVector = lPosition.xyz + vViewPosition.xyz;
++#else
+         vec3 lVector = lPosition.xyz + vViewPosition.xyz;
++#endif
+         float lDistance = 1.0;
+         if ( pointLightDistance[ i ] > 0.0 )
+             lDistance = 1.0 - min( ( length( lVector ) / pointLightDistance[ i ] ), 1.0 );
+
+From 1edd76fda77edabd49d713912aee49b8360c86c3 Mon Sep 17 00:00:00 2001
+From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+Date: Wed, 11 Nov 2020 09:49:52 -0500
+Subject: [PATCH 2/3] terrain: Fix precision handling in noise shader
+
+Another overflow resulting in infinity in mediump. Note this bug is
+masked if the driver clamps infinity to MAX_FLOAT, but it's still our
+bug.
+
+Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+---
+ data/shaders/terrain-noise.frag | 6 ++++++
+ 1 file changed, 6 insertions(+)
+
+diff --git a/data/shaders/terrain-noise.frag b/data/shaders/terrain-noise.frag
+index 7fea5c0..9535e58 100644
+--- a/data/shaders/terrain-noise.frag
++++ b/data/shaders/terrain-noise.frag
+@@ -17,7 +17,13 @@ uniform float time;
+ uniform MEDIUMP vec2 uvScale;
+ varying vec2 vUv;
+ 
++#ifdef GL_FRAGMENT_PRECISION_HIGH
++// x should be passed as highp since the intermediate multiplications can
++// overflow with mediump
++vec4 permute(highp vec4 x)
++#else
+ vec4 permute(vec4 x)
++#endif
+ {
+     return mod(((x * 34.0) + 1.0) * x, 289.0);
+ }
+
+From e866cc633ffc450e5358b2742f32ca360e4f3f12 Mon Sep 17 00:00:00 2001
+From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+Date: Wed, 11 Nov 2020 09:35:21 -0500
+Subject: [PATCH 3/3] loop,function,conditionals: Fix mediump overflow
+
+The multiplication can produce infinity.
+
+Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
+---
+ data/shaders/conditionals.frag | 9 ++++++++-
+ data/shaders/function.frag     | 9 ++++++++-
+ data/shaders/loop.frag         | 9 ++++++++-
+ 3 files changed, 24 insertions(+), 3 deletions(-)
+
+diff --git a/data/shaders/conditionals.frag b/data/shaders/conditionals.frag
+index 3bd2507..e902263 100644
+--- a/data/shaders/conditionals.frag
++++ b/data/shaders/conditionals.frag
+@@ -2,7 +2,14 @@ varying vec4 dummy;
+ 
+ void main(void)
+ {
+-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
++#ifdef GL_FRAGMENT_PRECISION_HIGH
++    // should be declared highp since the multiplication can overflow in
++    // mediump, particularly if mediump is implemented as fp16
++    highp vec2 FragCoord = gl_FragCoord.xy;
++#else
++    vec2 FragCoord = gl_FragCoord.xy;
++#endif
++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
+ 
+ $MAIN$
+ 
+diff --git a/data/shaders/function.frag b/data/shaders/function.frag
+index 3e3c74f..9d0230e 100644
+--- a/data/shaders/function.frag
++++ b/data/shaders/function.frag
+@@ -8,7 +8,14 @@ $PROCESS$
+ 
+ void main(void)
+ {
+-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
++#ifdef GL_FRAGMENT_PRECISION_HIGH
++    // should be declared highp since the multiplication can overflow in
++    // mediump, particularly if mediump is implemented as fp16
++    highp vec2 FragCoord = gl_FragCoord.xy;
++#else
++    vec2 FragCoord = gl_FragCoord.xy;
++#endif
++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
+ 
+ $MAIN$
+ 
+diff --git a/data/shaders/loop.frag b/data/shaders/loop.frag
+index 31ae23e..9a6afd2 100644
+--- a/data/shaders/loop.frag
++++ b/data/shaders/loop.frag
+@@ -3,7 +3,14 @@ uniform int FragmentLoops;
+ 
+ void main(void)
+ {
+-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
++#ifdef GL_FRAGMENT_PRECISION_HIGH
++    // should be declared highp since the multiplication can overflow in
++    // mediump, particularly if mediump is implemented as fp16
++    highp vec2 FragCoord = gl_FragCoord.xy;
++#else
++    vec2 FragCoord = gl_FragCoord.xy;
++#endif
++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
+ 
+ $MAIN$
+ 
diff --git a/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend
new file mode 100644
index 0000000..e7a7173
--- /dev/null
+++ b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend
@@ -0,0 +1,5 @@
+FILESEXTRAPATHS_prepend := "${THISDIR}/files:"
+
+SRC_URI_append_rock-pi-4 = " \
+	file://fix-precision-handling-bugs.patch \
+	"
-- 
2.30.0.rc0

Re: [yocto] [meta-rockchip][PATCH] glmark2: fix bugs on panfrost

[Khem Raj]

On 12/17/20 9:03 PM, Trevor Woerner wrote:
> Alyssa submitted this set of patches for glmark2 to fix/clarify glmark2's
> handling of fp16 overflows in a shader (which succeed if the driver doesn't
> handle fp16 and uses fp32 instead).
> 
> See: https://github.com/glmark2/glmark2/pull/132
> 
> This is added as a dynamic layer since meta-openembedded/meta-oe isn't a
> requirement for this layer.
> 
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
>   conf/layer.conf                               |   6 +
>   .../files/fix-precision-handling-bugs.patch   | 138 ++++++++++++++++++
>   .../glmark2/glmark2_%.bbappend                |   5 +
>   3 files changed, 149 insertions(+)
>   create mode 100644 dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
>   create mode 100644 dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend
> 
> diff --git a/conf/layer.conf b/conf/layer.conf
> index 8eecdc5..e4b3055 100644
> --- a/conf/layer.conf
> +++ b/conf/layer.conf
> @@ -16,3 +16,9 @@ BBFILE_PRIORITY_rockchip = "1"
>   LAYERVERSION_rockchip = "1"
>   LAYERSERIES_COMPAT_rockchip = "gatesgarth"
>   LAYERDEPENDS_rockchip = "core meta-arm"
> +
> +# dynamic layers
> +BBFILES += "${@' '.join('${LAYERDIR}/dynamic-layers/%s/recipes*/*/*.bbappend' % layer \
> +               for layer in BBFILE_COLLECTIONS.split())}"
> +BBFILES += "${@' '.join('${LAYERDIR}/dynamic-layers/%s/recipes*/*/*.bb' % layer \
> +               for layer in BBFILE_COLLECTIONS.split())}"
> diff --git a/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
> new file mode 100644
> index 0000000..af88f6c
> --- /dev/null
> +++ b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/files/fix-precision-handling-bugs.patch
> @@ -0,0 +1,138 @@
> +From 90e837ffd1ff5c9add1074d69de23e58a3a4810e Mon Sep 17 00:00:00 2001
> +From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +Date: Wed, 11 Nov 2020 09:26:03 -0500
> +Subject: [PATCH 1/3] terrain: Fix precision bug in light rendering
> +
> +Resulting in overly bright rendering when mediump is implemented as
> +fp16.
> +
> +Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +---
> + data/shaders/terrain.frag | 5 +++++
> + 1 file changed, 5 insertions(+)
> +
> +diff --git a/data/shaders/terrain.frag b/data/shaders/terrain.frag
> +index 84d085c..58f17ea 100644
> +--- a/data/shaders/terrain.frag
> ++++ b/data/shaders/terrain.frag
> +@@ -67,7 +67,12 @@ void main() {
> +     vec3 pointSpecular = vec3( 0.0 );
> +     for ( int i = 0; i < MAX_POINT_LIGHTS; i ++ ) {
> +         vec4 lPosition = viewMatrix * vec4( pointLightPosition[ i ], 1.0 );
> ++#ifdef GL_FRAGMENT_PRECISION_HIGH
> ++        // should be highp for correct behaviour if mediump is implemented as fp16
> ++        highp vec3 lVector = lPosition.xyz + vViewPosition.xyz;
> ++#else
> +         vec3 lVector = lPosition.xyz + vViewPosition.xyz;
> ++#endif
> +         float lDistance = 1.0;
> +         if ( pointLightDistance[ i ] > 0.0 )
> +             lDistance = 1.0 - min( ( length( lVector ) / pointLightDistance[ i ] ), 1.0 );
> +
> +From 1edd76fda77edabd49d713912aee49b8360c86c3 Mon Sep 17 00:00:00 2001
> +From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +Date: Wed, 11 Nov 2020 09:49:52 -0500
> +Subject: [PATCH 2/3] terrain: Fix precision handling in noise shader
> +
> +Another overflow resulting in infinity in mediump. Note this bug is
> +masked if the driver clamps infinity to MAX_FLOAT, but it's still our
> +bug.
> +
> +Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +---
> + data/shaders/terrain-noise.frag | 6 ++++++
> + 1 file changed, 6 insertions(+)
> +
> +diff --git a/data/shaders/terrain-noise.frag b/data/shaders/terrain-noise.frag
> +index 7fea5c0..9535e58 100644
> +--- a/data/shaders/terrain-noise.frag
> ++++ b/data/shaders/terrain-noise.frag
> +@@ -17,7 +17,13 @@ uniform float time;
> + uniform MEDIUMP vec2 uvScale;
> + varying vec2 vUv;
> +
> ++#ifdef GL_FRAGMENT_PRECISION_HIGH
> ++// x should be passed as highp since the intermediate multiplications can
> ++// overflow with mediump
> ++vec4 permute(highp vec4 x)
> ++#else
> + vec4 permute(vec4 x)
> ++#endif
> + {
> +     return mod(((x * 34.0) + 1.0) * x, 289.0);
> + }
> +
> +From e866cc633ffc450e5358b2742f32ca360e4f3f12 Mon Sep 17 00:00:00 2001
> +From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +Date: Wed, 11 Nov 2020 09:35:21 -0500
> +Subject: [PATCH 3/3] loop,function,conditionals: Fix mediump overflow
> +
> +The multiplication can produce infinity.
> +
> +Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> +---
> + data/shaders/conditionals.frag | 9 ++++++++-
> + data/shaders/function.frag     | 9 ++++++++-
> + data/shaders/loop.frag         | 9 ++++++++-
> + 3 files changed, 24 insertions(+), 3 deletions(-)
> +
> +diff --git a/data/shaders/conditionals.frag b/data/shaders/conditionals.frag
> +index 3bd2507..e902263 100644
> +--- a/data/shaders/conditionals.frag
> ++++ b/data/shaders/conditionals.frag
> +@@ -2,7 +2,14 @@ varying vec4 dummy;
> +
> + void main(void)
> + {
> +-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
> ++#ifdef GL_FRAGMENT_PRECISION_HIGH
> ++    // should be declared highp since the multiplication can overflow in
> ++    // mediump, particularly if mediump is implemented as fp16
> ++    highp vec2 FragCoord = gl_FragCoord.xy;
> ++#else
> ++    vec2 FragCoord = gl_FragCoord.xy;
> ++#endif
> ++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
> +
> + $MAIN$
> +
> +diff --git a/data/shaders/function.frag b/data/shaders/function.frag
> +index 3e3c74f..9d0230e 100644
> +--- a/data/shaders/function.frag
> ++++ b/data/shaders/function.frag
> +@@ -8,7 +8,14 @@ $PROCESS$
> +
> + void main(void)
> + {
> +-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
> ++#ifdef GL_FRAGMENT_PRECISION_HIGH
> ++    // should be declared highp since the multiplication can overflow in
> ++    // mediump, particularly if mediump is implemented as fp16
> ++    highp vec2 FragCoord = gl_FragCoord.xy;
> ++#else
> ++    vec2 FragCoord = gl_FragCoord.xy;
> ++#endif
> ++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
> +
> + $MAIN$
> +
> +diff --git a/data/shaders/loop.frag b/data/shaders/loop.frag
> +index 31ae23e..9a6afd2 100644
> +--- a/data/shaders/loop.frag
> ++++ b/data/shaders/loop.frag
> +@@ -3,7 +3,14 @@ uniform int FragmentLoops;
> +
> + void main(void)
> + {
> +-    float d = fract(gl_FragCoord.x * gl_FragCoord.y * 0.0001);
> ++#ifdef GL_FRAGMENT_PRECISION_HIGH
> ++    // should be declared highp since the multiplication can overflow in
> ++    // mediump, particularly if mediump is implemented as fp16
> ++    highp vec2 FragCoord = gl_FragCoord.xy;
> ++#else
> ++    vec2 FragCoord = gl_FragCoord.xy;
> ++#endif
> ++    float d = fract(FragCoord.x * FragCoord.y * 0.0001);
> +
> + $MAIN$
> +
> diff --git a/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend
> new file mode 100644
> index 0000000..e7a7173
> --- /dev/null
> +++ b/dynamic-layers/openembedded-layer/recipes-benchmark/glmark2/glmark2_%.bbappend
> @@ -0,0 +1,5 @@
> +FILESEXTRAPATHS_prepend := "${THISDIR}/files:"
> +
> +SRC_URI_append_rock-pi-4 = " \
> +	file://fix-precision-handling-bugs.patch \
> +	"
> 

this patch is generic, perhaps should live with main recipe.
> 
> 
> 
> 

Re: [yocto] [meta-rockchip][PATCH] glmark2: fix bugs on panfrost

[Trevor Woerner]

On Fri 2020-12-18 @ 08:36:28 AM, Khem Raj wrote:
> this patch is generic, perhaps should live with main recipe.

Okay.