diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f3984cf7..efc8aaf3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -577,11 +577,22 @@ jobs: # An arm whose subsystem has not landed on this tree SKIPS with the reason (never absent, # never a green that asserted nothing), so this step is green through the P2 landing order # and starts asserting as each package arrives. + # + # The environment is the sibling step's, deliberately and in full: these entries run the + # same DirectVulkan binary through the same runner, so the three MOBILEGL_MAGMA_* fixes it + # needs apply here too, and a crash here has to leave a core for the same black-box flow. + # The step above is the only reason those lines exist in this job; a control that crashed + # without one would be the hardest failure in the job to diagnose. - name: The handle-ABA and CSO-content-addressing controls (G8, G12) working-directory: build-verify env: MOBILEGL_ITEST_REQUIRE_GPU: "1" + MOBILEGL_MAGMA_FIX_ITERATIONRP_SUBGROUP_SCRATCH: "1" + MOBILEGL_MAGMA_DERIVE_NUM_SUBGROUPS: "1" + MOBILEGL_MAGMA_ITERATIONRP_FIX_BARRIER: "1" run: | + ulimit -c unlimited + sudo sysctl -w kernel.core_pattern='/tmp/core.%e.%p' ctest --output-on-failure -L integration-gpu \ -R 'HandleRecycle|CsoContentAddressing' --no-tests=error -j 4 diff --git a/tools/device_bench/README.md b/tools/device_bench/README.md index 17783169..a83fcaa7 100644 --- a/tools/device_bench/README.md +++ b/tools/device_bench/README.md @@ -35,6 +35,9 @@ frequency-pin integrity. (`big_cur`/`little_cur`/`gpu_cur_khz` in the result JSON must match the pins). Until then it says `PROFILE_VERIFIED=0` and `bench.sh` / `session.sh` refuse to run against it unless `--allow-unverified-profile` is passed, which labels the run unpinned in the warning. + **A profile that omits the key entirely is refused the same way** - the guard defaults to + unverified, so copying a verified profile and editing the serial cannot inherit its verdict. + (`profile.sh` pins nothing - it records a simpleperf profile - so it carries no such guard.) That refusal exists because the pin path is silent when it is wrong: the harness writes through `/proc/ppm/policy/hard_userlimit_*` and `/proc/gpufreq/gpufreq_opp_freq`, which are diff --git a/tools/device_bench/bench.sh b/tools/device_bench/bench.sh index 29bda024..6410ce3f 100755 --- a/tools/device_bench/bench.sh +++ b/tools/device_bench/bench.sh @@ -69,15 +69,20 @@ done # TYPE off THIS device, ran one pinned window, and checked big_cur/little_cur/gpu_cur_khz in the # result JSON against the pins. Nothing else earns it. require_verified_profile() { - if [ "${PROFILE_VERIFIED:-1}" = "1" ]; then return 0; fi + # The default is UNVERIFIED. A profile that simply omits the key is a profile nobody has + # confirmed against its device, and defaulting it to "verified" would hand exactly the + # fail-open behaviour this guard exists to prevent to the most likely way a new profile is + # written - by copying an existing one and editing the serial. + if [ "${PROFILE_VERIFIED:-0}" = "1" ]; then return 0; fi if [ "$ALLOW_UNVERIFIED_PROFILE" = "1" ]; then - echo "[warn] $DEVICE_ENV declares PROFILE_VERIFIED=0 and --allow-unverified-profile was passed:" >&2 + echo "[warn] $DEVICE_ENV does not carry PROFILE_VERIFIED=1 and --allow-unverified-profile was passed:" >&2 echo "[warn] the frequency pins and the thermal gate in it are UNCONFIRMED, so any number this" >&2 echo "[warn] run produces is not comparable with a pinned one." >&2 return 0 fi - echo "$DEVICE_ENV declares PROFILE_VERIFIED=0: its sysfs nodes and OPPs have not been read off" >&2 - echo "the device, so pinning would fail silently and the run would look pinned but not be." >&2 + echo "$DEVICE_ENV does not carry PROFILE_VERIFIED=1 (it says 0, or says nothing at all): its" >&2 + echo "sysfs nodes and OPPs have not been read off the device, so pinning would fail silently" >&2 + echo "and the run would look pinned but not be." >&2 echo "Fill in the TODO_VERIFY_ON_DEVICE fields, confirm one pinned window, set PROFILE_VERIFIED=1 -" >&2 echo "or pass --allow-unverified-profile to measure anyway and label the result unpinned." >&2 exit 2 diff --git a/tools/device_bench/devices/odinlite.env b/tools/device_bench/devices/odinlite.env index ff4ce650..bc3b22ea 100644 --- a/tools/device_bench/devices/odinlite.env +++ b/tools/device_bench/devices/odinlite.env @@ -3,6 +3,12 @@ # Panel: 1080x1920 @ 60Hz (presented FPS caps at 60 - render-side FPS comes from the # FCLFPS logcat tag, which counts eglSwapBuffers; it is NOT vsync-capped when the # game runs with vsync off). +# Read off this device and confirmed against one pinned window (big_cur/little_cur/gpu_cur_khz +# in the result JSON matched the pins), which is what earns the key. bench.sh / session.sh refuse +# a profile without it: the default is UNVERIFIED, so a profile written by copying this one starts +# out refused until somebody repeats that check on the new device. +PROFILE_VERIFIED=1 + DEVICE_SERIAL=MTK0002207301023500 # Frequency pins, enforced via /proc/ppm/policy/hard_userlimit_* (plain cpufreq diff --git a/tools/device_bench/devices/oppo-mali.env b/tools/device_bench/devices/oppo-mali.env index 69e81ec1..323b6387 100644 --- a/tools/device_bench/devices/oppo-mali.env +++ b/tools/device_bench/devices/oppo-mali.env @@ -5,8 +5,9 @@ # repository rather than in one operator's shell history. # # ============================ NOT YET DEVICE-VERIFIED ============================ -# PROFILE_VERIFIED=0, and bench.sh / session.sh / profile.sh refuse to run against it unless -# --allow-unverified-profile is passed. This part is a MediaTek SoC, so unlike the Adreno +# PROFILE_VERIFIED=0, and bench.sh / session.sh refuse to run against it - or against a profile +# that omits the key - unless --allow-unverified-profile is passed. (profile.sh only records a +# simpleperf profile and pins nothing, so it carries no such guard.) This part is a MediaTek SoC, so unlike the Adreno # profile the harness's existing /proc/ppm + /proc/gpufreq pin path is probably the right one - # but "probably" is exactly the state a measurement profile must not ship in. The cluster # indices, the available OPPs, the top GPU OPP and the thermal zone TYPE all differ between diff --git a/tools/device_bench/devices/xiaomi-adreno830.env b/tools/device_bench/devices/xiaomi-adreno830.env index 360a5407..a1c2ad92 100644 --- a/tools/device_bench/devices/xiaomi-adreno830.env +++ b/tools/device_bench/devices/xiaomi-adreno830.env @@ -6,8 +6,9 @@ # history, and so that a `--device` argument names something reviewable. # # ============================ NOT YET DEVICE-VERIFIED ============================ -# PROFILE_VERIFIED=0 below, and bench.sh / session.sh / profile.sh REFUSE to run against a -# profile that says so unless --allow-unverified-profile is passed. Two of the values here are +# PROFILE_VERIFIED=0 below, and bench.sh / session.sh REFUSE to run against a profile that says +# so - or that omits the key - unless --allow-unverified-profile is passed. (profile.sh is not in +# that list: it records a simpleperf profile and pins nothing, so it has nothing to pin wrongly.) Two of the values here are # protocol constants that are known (the campaign pins big 1.96 GHz / little 1.55 GHz and gates # at 40 C), but the sysfs node names and the exact available OPPs are NOT: this is a Qualcomm # part and the harness was written against MediaTek, where the pin goes through diff --git a/tools/device_bench/session.sh b/tools/device_bench/session.sh index 44040fd4..80323b71 100755 --- a/tools/device_bench/session.sh +++ b/tools/device_bench/session.sh @@ -52,15 +52,20 @@ done # TYPE off THIS device, ran one pinned window, and checked big_cur/little_cur/gpu_cur_khz in the # result JSON against the pins. Nothing else earns it. require_verified_profile() { - if [ "${PROFILE_VERIFIED:-1}" = "1" ]; then return 0; fi + # The default is UNVERIFIED. A profile that simply omits the key is a profile nobody has + # confirmed against its device, and defaulting it to "verified" would hand exactly the + # fail-open behaviour this guard exists to prevent to the most likely way a new profile is + # written - by copying an existing one and editing the serial. + if [ "${PROFILE_VERIFIED:-0}" = "1" ]; then return 0; fi if [ "$ALLOW_UNVERIFIED_PROFILE" = "1" ]; then - echo "[warn] $DEVICE_ENV declares PROFILE_VERIFIED=0 and --allow-unverified-profile was passed:" >&2 + echo "[warn] $DEVICE_ENV does not carry PROFILE_VERIFIED=1 and --allow-unverified-profile was passed:" >&2 echo "[warn] the frequency pins and the thermal gate in it are UNCONFIRMED, so any number this" >&2 echo "[warn] run produces is not comparable with a pinned one." >&2 return 0 fi - echo "$DEVICE_ENV declares PROFILE_VERIFIED=0: its sysfs nodes and OPPs have not been read off" >&2 - echo "the device, so pinning would fail silently and the run would look pinned but not be." >&2 + echo "$DEVICE_ENV does not carry PROFILE_VERIFIED=1 (it says 0, or says nothing at all): its" >&2 + echo "sysfs nodes and OPPs have not been read off the device, so pinning would fail silently" >&2 + echo "and the run would look pinned but not be." >&2 echo "Fill in the TODO_VERIFY_ON_DEVICE fields, confirm one pinned window, set PROFILE_VERIFIED=1 -" >&2 echo "or pass --allow-unverified-profile to measure anyway and label the result unpinned." >&2 exit 2 diff --git a/tools/trace_replay/run_android_retrace_local.py b/tools/trace_replay/run_android_retrace_local.py index 22c15c7d..9d48a919 100644 --- a/tools/trace_replay/run_android_retrace_local.py +++ b/tools/trace_replay/run_android_retrace_local.py @@ -226,12 +226,32 @@ def read_benchmark(case, backend, run_index): return report +def series_median(values): + """The median, by the rule SummarizeSeries uses on the device. + + trace_replay_core.cpp's SeriesSummary takes the middle element of an odd window and the + AVERAGE of the two middle elements of an even one, so p50 has to be computed the same way or + the line would print a p50 next to a medianFrameCpuMs that disagreed with it for a reason + nobody could see. (It is the only one of the three that is not a nearest rank: the device's + p95 is.) + """ + if not values: + return -1.0 + ordered = sorted(values) + middle = len(ordered) // 2 + if len(ordered) % 2 == 1: + return ordered[middle] + return 0.5 * (ordered[middle - 1] + ordered[middle]) + + def nearest_rank_percentile(values, fraction): - """Nearest-rank percentile, the same rule SummarizeSeries uses on the device. + """Nearest-rank percentile, the rule SummarizeSeries uses on the device for p95. Nearest rank rather than an interpolating percentile so that every number printed here is a frame that was actually observed, and so that a p95 computed on this side agrees exactly with - the p95 the device reported for the same window. + the p95 the device reported for the same window. The device computes no p99 at all - that is + the whole reason benchmark.json carries the full series - so p99 is this rule extended, and + p50 is NOT computed here (see series_median). """ if not values: return -1.0 @@ -245,11 +265,12 @@ def nearest_rank_percentile(values, fraction): def cpu_tail(report): """The trailing window of the per-frame CPU series, or [] when the run collected none. - benchmark.json carries the WHOLE frameCpuTimesMs[] array precisely so that percentiles the - device does not compute - p50 and p99, which are what the paired A/B publishes - are a - host-side reduction over an artefact that already exists. The window is the same trailing - tailFrames the device summarised, so the numbers below sit beside the device's own without - being about a different set of frames. + benchmark.json carries the WHOLE frameCpuTimesMs[] array precisely so that p99 - which the + device does not compute, and which the paired A/B publishes beside p50 - is a host-side + reduction over an artefact that already exists. The window is the same trailing tailFrames the + device summarised, so the numbers below sit beside the device's own without being about a + different set of frames; p50 is recomputed here by the device's own median rule, so it agrees + with medianFrameCpuMs on the same run rather than merely sitting next to it. """ series = report.get("frameCpuTimesMs") or [] if not series: @@ -277,7 +298,7 @@ def format_benchmark(report): if window: line += ( f" | cpu mean={report.get('meanFrameCpuMs', -1):.3f}ms" - f" p50={nearest_rank_percentile(window, 0.50):.3f}ms" + f" p50={series_median(window):.3f}ms" f" p95={report.get('p95FrameCpuMs', -1):.3f}ms" f" p99={nearest_rank_percentile(window, 0.99):.3f}ms" )