From ca54cc817aa0f3b1e63ba24c6200df599f35750c Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Tue, 13 Oct 2026 14:35:54 +1100 Subject: [PATCH] drm/apple: reject unimplemented external H17P VRR External H17P mode parsing copies the sink adaptive-sync range into modes, so get_modes advertises VRR capability. The external modeset path skips adaptive-sync programming, but atomic checks still accept VRR_ENABLED. Keep those timings available as fixed refresh modes without exposing VRR, and reject external H17P atomic requests that enable it. Preserve legacy and internal mode parsing. Exercise the production binary parser and atomic admission check, including plane-only commits and link recovery. Signed-off-by: Ryan Murray Assisted-by: LLM --- MAINTAINERS | 1 + drivers/gpu/drm/apple/dcp.c | 6 ++ drivers/gpu/drm/apple/parser.c | 8 +- tools/diagnostics/test-apple-external-sdr.py | 15 +++- tools/diagnostics/test-apple-external-vrr.py | 93 ++++++++++++++++++++ 5 files changed, 117 insertions(+), 6 deletions(-) create mode 100755 tools/diagnostics/test-apple-external-vrr.py diff --git a/MAINTAINERS b/MAINTAINERS index f3d453a49eb90..e628a60420a24 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1952,6 +1952,7 @@ L: dri-devel@lists.freedesktop.org S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc F: drivers/gpu/drm/apple/ +F: tools/diagnostics/test-apple-external-vrr.py F: tools/testing/selftests/drivers/gpu/apple-afkv2/ APPLE PCIE CONTROLLER DRIVER diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index 20115ce416842..ab80ad9ce0255 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -2334,6 +2334,12 @@ int dcp_crtc_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state) crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + /* External H17P does not program adaptive sync. */ + if (dcp->hw.iomfb_method_profile == + DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL && + crtc_state->vrr_enabled) + return -EINVAL; + needs_modeset = drm_atomic_crtc_needs_modeset(crtc_state) || !READ_ONCE(dcp->mode_state.valid); if (!needs_modeset && (!dcp->connector || !dcp->connector->connected)) { diff --git a/drivers/gpu/drm/apple/parser.c b/drivers/gpu/drm/apple/parser.c index 8c2a1069417b6..c53afe96926d9 100644 --- a/drivers/gpu/drm/apple/parser.c +++ b/drivers/gpu/drm/apple/parser.c @@ -765,8 +765,12 @@ static int parse_mode(struct dcp_parse_ctx *handle, out->vrr = true; } - /* Refresh rates are reported by DCP as 16.16 fixed-point Hz. */ - if (min_vrr && max_vrr > min_vrr) { + /* + * Refresh rates are reported by DCP as 16.16 fixed-point Hz. + * External H17P skips adaptive-sync programming: keep the fixed timing + * available, but do not advertise its sink VRR range as driver support. + */ + if (!external_sdr && min_vrr && max_vrr > min_vrr) { out->min_vrr = min_vrr; out->max_vrr = max_vrr; out->vrr = true; diff --git a/tools/diagnostics/test-apple-external-sdr.py b/tools/diagnostics/test-apple-external-sdr.py index 695a6059da419..d81ac74c9799a 100644 --- a/tools/diagnostics/test-apple-external-sdr.py +++ b/tools/diagnostics/test-apple-external-sdr.py @@ -74,10 +74,10 @@ def timing(colors, rate=60, identity=203, score=10): cases = [] -def case(name, timings, external=True, count=1, identity=9, depth=8, clock=148500): +def case(name, timings, external=True, count=1, identity=9, depth=8, clock=148500, vrr=False): out = bytearray(struct.pack(' +#include +#include +#include +#define DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL 2 +#define READ_ONCE(x) (x) +#define dev_dbg(...) ((void)0) +struct apple_connector { bool connected; }; +struct apple_dcp { + bool crashed; + struct { int iomfb_method_profile; } hw; + struct { bool valid; } mode_state; + struct apple_connector *connector; + void *dev; +}; +struct platform_device { struct apple_dcp *data; }; +struct drm_crtc { struct platform_device *dcp; }; +struct drm_crtc_state { bool vrr_enabled, modeset; }; +struct drm_atomic_state { struct drm_crtc_state crtc; }; +#define to_apple_crtc(x) (x) +#define platform_get_drvdata(x) ((x)->data) +static struct drm_crtc_state *drm_atomic_get_new_crtc_state( + struct drm_atomic_state *s, struct drm_crtc *c) +{ (void)c; return &s->crtc; } +static bool drm_atomic_crtc_needs_modeset(struct drm_crtc_state *s) +{ return s->modeset; } +""" +suffix = r""" +int main(void) +{ + struct apple_connector connector = { .connected = true }; + struct apple_dcp dcp = { .connector = &connector }; + struct platform_device pdev = { .data = &dcp }; + struct drm_crtc crtc = { .dcp = &pdev }; + struct drm_atomic_state state = {0}; + unsigned int cases = 0; + /* Check initial modesets, plane-only commits and link recovery states. */ + for (unsigned int profile = 0; profile < 3; profile++) + for (unsigned int valid = 0; valid < 2; valid++) + for (unsigned int modeset = 0; modeset < 2; modeset++) + for (unsigned int link = 0; link < 3; link++) + for (unsigned int vrr = 0; vrr < 2; vrr++) { + dcp.hw.iomfb_method_profile = profile; + dcp.mode_state.valid = valid; + state.crtc.modeset = modeset; + state.crtc.vrr_enabled = vrr; + dcp.connector = link == 2 ? NULL : &connector; + connector.connected = link == 1; + int expected = profile == DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL && vrr ? -EINVAL : 0; + int actual = dcp_crtc_atomic_check(&crtc, &state); + if (actual != expected) { + fprintf(stderr, "profile=%u valid=%u modeset=%u link=%u vrr=%u: got %d expected %d\n", + profile, valid, modeset, link, vrr, actual, expected); + return 1; + } + cases++; + } + dcp.crashed = true; + state.crtc.vrr_enabled = false; + assert(dcp_crtc_atomic_check(&crtc, &state) == -EINVAL); + printf("PASS: %u production atomic VRR admission cases and crashed-device rejection\n", cases); + return 0; +} +""" +with tempfile.TemporaryDirectory(prefix='apple-external-vrr-') as tmp: + tmp = Path(tmp) + unit = tmp / 'test.c' + unit.write_text(prefix + check + suffix) + command = shlex.split(os.environ.get('CC', 'clang')) + command += ['-std=gnu11', '-Wall', '-Wextra', '-Werror', '-O1', '-g', + '-fsanitize=address,undefined', '-fno-omit-frame-pointer'] + subprocess.run(command + [str(unit), '-o', str(tmp / 'test')], check=True) + subprocess.run([str(tmp / 'test')], check=True)