From 35f4ccf26053b10c3f9750a218b9328239bd8eec Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Mon, 12 Oct 2026 19:59:19 +1100 Subject: [PATCH 1/4] drm/apple: add bounded external stop qualification path Pause new presentation work while callbacks and control RPCs remain available; settle the active queue, serialize external lifecycle operations and retain resources on failure. Preserve first-start preparation and generation-aware CRTC recovery. OFF replies are captured but deliberately return EOPNOTSUPP until their contract is qualified; no successful production teardown or DMA retirement is claimed. Host tests exercise admission, drain, exact response bounds, timeout ownership and lifecycle ordering. A hypothetical qualified-OFF adapter exists only inside the host test. Astra reviewed this revision for one initialized DPMS-OFF capture followed by reboot, excluding unplug/suspend/restart qualification. Generated-by: LLM Assisted-by: LLM Signed-off-by: Ryan Murray --- MAINTAINERS | 1 + drivers/gpu/drm/apple/apple_drv.c | 3 + drivers/gpu/drm/apple/dcp-internal.h | 25 + drivers/gpu/drm/apple/dcp.c | 281 +++++++++- drivers/gpu/drm/apple/dcp.h | 1 + drivers/gpu/drm/apple/iomfb.c | 64 ++- drivers/gpu/drm/apple/iomfb_h17p.c | 99 ++++ drivers/gpu/drm/apple/iomfb_template.c | 3 + .../test-apple-external-lifecycle.py | 529 ++++++++++++++++++ 9 files changed, 999 insertions(+), 7 deletions(-) create mode 100644 tools/diagnostics/test-apple-external-lifecycle.py diff --git a/MAINTAINERS b/MAINTAINERS index 9e9fbe808e1bc..508097a2e7db3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2021,6 +2021,7 @@ F: tools/diagnostics/apple-j700-dp-test.c F: tools/diagnostics/test-apple-afk-admission.py F: tools/diagnostics/test-apple-atc-unwind.py F: tools/diagnostics/test-apple-dcp-readiness.py +F: tools/diagnostics/test-apple-external-lifecycle.py F: tools/diagnostics/test-apple-j700-crossbar.py F: tools/diagnostics/test-apple-j700-d134.py F: tools/diagnostics/test-apple-j700-dp.py diff --git a/drivers/gpu/drm/apple/apple_drv.c b/drivers/gpu/drm/apple/apple_drv.c index d0462690c25c2..9a98818391aa2 100644 --- a/drivers/gpu/drm/apple/apple_drv.c +++ b/drivers/gpu/drm/apple/apple_drv.c @@ -152,6 +152,9 @@ static void apple_crtc_atomic_enable(struct drm_crtc *crtc, dcp_poweron(apple_crtc->dcp); /* Force the CTM to be set on first swap */ crtc_state->color_mgmt_changed = true; + } else if (crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state)) { + /* A cable generation can change while the CRTC stays active. */ + dcp_external_recover(to_apple_crtc(crtc)->dcp); } if (crtc_state->active) diff --git a/drivers/gpu/drm/apple/dcp-internal.h b/drivers/gpu/drm/apple/dcp-internal.h index 48f4112151dd3..685c985680ba0 100644 --- a/drivers/gpu/drm/apple/dcp-internal.h +++ b/drivers/gpu/drm/apple/dcp-internal.h @@ -46,6 +46,7 @@ struct iomfb_transaction { bool backlight_reserved; bool backlight_failed; bool brightness_only; + bool presentation; bool completed; }; struct apple_dcp_typec_port; @@ -213,6 +214,25 @@ struct apple_dcp { struct apple_dcp_hw_data hw; bool external_prepared; + /* Serializes external connection, power, and CRTC generation changes. */ + struct mutex external_lock; + u64 external_generation; + u64 external_crtc_generation; + /* iomfb.lock: epoch used by receive-side deferred stop requests. */ + u64 external_rx_generation; + u64 external_stop_generation; + bool external_cable; + bool external_running; + bool external_power_attempted; + /* iomfb.lock: replay the latest callback after activation succeeds. */ + bool external_hotplug_pending; + bool external_hotplug_connected; + bool external_stopped; + int external_error; + u64 external_power_reply; + bool external_power_reply_valid; + struct work_struct external_stop_work; + /* firmware version and compatible firmware version */ enum dcp_firmware_version fw_compat; @@ -445,6 +465,7 @@ struct apple_dcp { unsigned long deadline; unsigned int queued; bool stopped; + bool presentation_paused; bool backlight_queued; atomic_t opaque_x_state; struct iomfb_scanout_h17p *scanout; @@ -469,11 +490,15 @@ int iomfb_configure_backlight_h17p(struct apple_dcp *dcp, u32 maximum, void iomfb_queue_init(struct apple_dcp *dcp); void iomfb_queue_stop(struct apple_dcp *dcp); bool iomfb_queue_drain(struct apple_dcp *dcp, unsigned long timeout); +int iomfb_queue_settle(struct apple_dcp *dcp, unsigned long timeout); +void iomfb_pause_presentation(struct apple_dcp *dcp, bool pause); +int iomfb_external_power_h17p(struct apple_dcp *dcp, bool on); int iomfb_queue(struct apple_dcp *dcp, struct iomfb_transaction *transaction); void iomfb_queue_crc_h17p(struct apple_dcp *dcp, u32 swap_id); void dcp_drm_crtc_page_flip(struct apple_dcp *dcp, ktime_t now); void dcp_handle_hotplug_actions(struct apple_dcp *dcp, unsigned int action); +bool dcp_external_hotplug(struct apple_dcp *dcp, bool connected); int dcp_backlight_init(struct apple_dcp *dcp, struct device *dev); void dcp_backlight_stop(struct apple_dcp *dcp); diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index 4f2b4923135af..da176a910f46b 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -301,6 +301,14 @@ static bool dcp_typec_route_fits(struct apple_dcp_typec_route *route, return connector->candidate_crtcs & drm_crtc_mask(&dcp->crtc->base); } +static bool dcp_is_external_h17p(struct apple_dcp *dcp) +{ + return dcp->hw.iomfb_method_profile == DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL; +} + +static int dcp_external_stop_locked(struct apple_dcp *dcp, u64 generation); +static int dcp_external_disconnect(struct apple_dcp *dcp, bool physical); + static int dcp_typec_route_activate(struct apple_dcp_typec_route *route, struct mux_control *xbar); static int dcp_typec_route_deactivate(struct apple_dcp_typec_route *route); @@ -349,7 +357,9 @@ static int dcp_typec_route_activate(struct apple_dcp_typec_route *route, * can still be marked connected to a display that is gone. Release it * (a no-op otherwise), or connecting the borrowed route returns early. */ - dcp_dptx_disconnect(dcp, 0); + ret = dcp_dptx_disconnect(dcp, 0); + if (ret) + return ret; if (dcp->fixed_route_selected) { ret = mux_control_deselect(dcp->xbar); @@ -439,6 +449,11 @@ static int dcp_typec_route_deactivate(struct apple_dcp_typec_route *route) bool was_tunnel = route->tunnel; int ret = 0; + guard(mutex)(&dcp->external_lock); + if (dcp_is_external_h17p(dcp) && + (dcp->external_error || (dcp->external_power_attempted && !dcp->external_stopped))) + return dcp->external_error ?: -EBUSY; + /* * Under tb_lock so a DCP link (re)configuration can neither select the * crossbar nor restart the tunnel pixel clock behind our back. @@ -1557,7 +1572,8 @@ static void dcp_typec_route_unregister(void *data) if (port->hpd || dcp->typec_cable_connected) dcp_dptx_disconnect_oob(to_platform_device(dcp->dev), 0); port->hpd = false; - dcp_typec_route_deactivate(route); + if (dcp_typec_route_deactivate(route) && dcp_is_external_h17p(dcp)) + return; port->owner = NULL; } if (port->secondary_owner == route) { @@ -1565,7 +1581,8 @@ static void dcp_typec_route_unregister(void *data) if (port->hpd || dcp->typec_cable_connected) dcp_dptx_disconnect_oob(to_platform_device(dcp->dev), 0); - dcp_typec_route_deactivate(route); + if (dcp_typec_route_deactivate(route) && dcp_is_external_h17p(dcp)) + return; port->secondary_owner = NULL; } port->hpd = !!(port->owner || port->secondary_owner); @@ -2171,7 +2188,7 @@ bool dcp_has_typec_routes(struct platform_device *pdev) #define DPTX_RECONNECT_DELAY msecs_to_jiffies(1000) #define DPTX_RECONNECT_RETRIES 5 -static int dcp_dptx_connect(struct apple_dcp *dcp, u32 port) +static int dcp_dptx_connect_raw(struct apple_dcp *dcp, u32 port) { unsigned long timeout; int ret = 0; @@ -2280,6 +2297,186 @@ static int dcp_dptx_connect(struct apple_dcp *dcp, u32 port) return ret; } +static void dcp_external_report_disconnected(struct apple_dcp *dcp) +{ + unsigned int action; + + guard(mutex)(&dcp->iomfb.lock); + action = dcp_mode_hotplug(&dcp->mode_state, false, + dcp->connector ? &dcp->connector->connected : NULL); + /* The existing worker delivers DRM notifications outside our locks. */ + dcp_handle_hotplug_actions(dcp, action); +} + +static void dcp_external_publish_generation(struct apple_dcp *dcp) +{ + lockdep_assert_held(&dcp->external_lock); + guard(mutex)(&dcp->iomfb.lock); + dcp->external_rx_generation = dcp->external_generation; +} + +/* external_lock is never taken by the serialized IOMFB receive callbacks. */ +static int dcp_external_link_locked(struct apple_dcp *dcp) +{ + int ret; + + lockdep_assert_held(&dcp->external_lock); + if (!dcp->external_prepared) + return -EAGAIN; + if (dcp->external_error) + return dcp->external_error; + if (!dcp->external_cable) + return -ENODEV; + ret = dcp_dptx_connect_raw(dcp, 0); + if (ret) + dcp->external_error = ret; + return ret; +} + +static int dcp_external_start_locked(struct apple_dcp *dcp) +{ + int ret; + + lockdep_assert_held(&dcp->external_lock); + ret = dcp_external_link_locked(dcp); + if (ret || dcp->external_running) + return ret; + iomfb_pause_presentation(dcp, true); + /* Even a timed-out first activation can have changed firmware state. */ + dcp->external_power_attempted = true; + ret = iomfb_external_power_h17p(dcp, true); + if (ret) { + dcp->external_error = ret; + return ret; + } + dcp->external_running = true; + dcp->external_stopped = false; + WRITE_ONCE(dcp->pipe_enabled_h17p, true); + WRITE_ONCE(dcp->typec_crtc_off, false); + iomfb_pause_presentation(dcp, false); + return 0; +} + +static int dcp_dptx_connect(struct apple_dcp *dcp, u32 port) +{ + if (!dcp_is_external_h17p(dcp)) + return dcp_dptx_connect_raw(dcp, port); + guard(mutex)(&dcp->external_lock); + return dcp_external_link_locked(dcp); +} + +/* The generation token belongs to the CRTC that was actually enabled. */ +static int dcp_external_stop_locked(struct apple_dcp *dcp, u64 generation) +{ + int ret; + + lockdep_assert_held(&dcp->external_lock); + if (generation != dcp->external_generation) + return 0; + if (dcp->external_error) + return dcp->external_error; + if (dcp->external_stopped) + return 0; + iomfb_pause_presentation(dcp, true); + dcp->external_running = false; + /* Route setup can call this before IOMFB or its shared memory exists. */ + if (!dcp->external_power_attempted) + goto release_link; + ret = iomfb_queue_settle(dcp, msecs_to_jiffies(500)); + if (!ret) + ret = iomfb_external_power_h17p(dcp, false); + if (!ret) + ret = iomfb_queue_settle(dcp, msecs_to_jiffies(500)); + if (ret) + goto failed; + /* This is a transport acknowledgment, not permission to free scanout. */ + WRITE_ONCE(dcp->pipe_enabled_h17p, false); +release_link: + scoped_guard(mutex, &dcp->hpd_mutex) { + if (dcp->dptxport[0].connected) { + if (!dptx_service_available(&dcp->dptxport[0])) { + ret = -ENODEV; + goto failed; + } + ret = dptxport_set_hpd(dcp->dptxport[0].service, false); + if (!ret) + ret = dptxport_release_display(dcp->dptxport[0].service); + if (ret) + goto failed; + dcp->dptxport[0].connected = false; + } + } + dcp->external_stopped = true; + WRITE_ONCE(dcp->typec_crtc_off, true); + if (dcp->avep) + av_service_disconnect(dcp); + return 0; +failed: + /* A retry could race a late reply. Keep this session blocked and owned. */ + dcp->external_error = ret; + return ret; +} + +static int dcp_external_disconnect(struct apple_dcp *dcp, bool physical) +{ + int ret; + + cancel_delayed_work_sync(&dcp->typec_reconnect_wq); + cancel_delayed_work_sync(&dcp->placeholder_edid_wq); + guard(mutex)(&dcp->external_lock); + ret = dcp_external_stop_locked(dcp, dcp->external_generation); + if (physical) { + dcp->external_cable = false; + dcp->external_generation++; + dcp_external_publish_generation(dcp); + scoped_guard(mutex, &dcp->hpd_mutex) { + WRITE_ONCE(dcp->typec_cable_connected, false); + dcp->typec_generation++; + } + /* A missing cable is reported even when its hardware remains owned. */ + dcp_external_report_disconnected(dcp); + } + return ret; +} + +static void dcp_external_stop_work(struct work_struct *work) +{ + struct apple_dcp *dcp = container_of(work, struct apple_dcp, external_stop_work); + + u64 generation; + + scoped_guard(mutex, &dcp->iomfb.lock) + generation = dcp->external_stop_generation; + cancel_delayed_work_sync(&dcp->typec_reconnect_wq); + cancel_delayed_work_sync(&dcp->placeholder_edid_wq); + guard(mutex)(&dcp->external_lock); + if (generation == dcp->external_generation) { + dcp_external_stop_locked(dcp, generation); + dcp_external_report_disconnected(dcp); + } +} + +bool dcp_external_hotplug(struct apple_dcp *dcp, bool connected) +{ + if (!dcp_is_external_h17p(dcp)) + return false; + /* No lifecycle or HPD mutex may be acquired from the RX callback. */ + lockdep_assert_held(&dcp->iomfb.lock); + if (READ_ONCE(dcp->quiescing)) + return true; + if (dcp->iomfb.presentation_paused) { + dcp->external_hotplug_pending = true; + dcp->external_hotplug_connected = connected; + return true; + } + if (!connected && !READ_ONCE(dcp->iomfb.presentation_paused)) { + dcp->external_stop_generation = dcp->external_rx_generation; + schedule_work(&dcp->external_stop_work); + return true; + } + return READ_ONCE(dcp->iomfb.presentation_paused); +} + static bool dcp_edid_is_placeholder(const struct drm_edid *drm_edid) { const u8 *raw = (const u8 *)drm_edid_raw(drm_edid); @@ -2397,6 +2594,9 @@ static void disconnected_hpd_event(struct apple_connector *con) static int dcp_dptx_disconnect(struct apple_dcp *dcp, u32 port) { + if (dcp_is_external_h17p(dcp)) + return dcp_external_disconnect(dcp, false); + dev_info(dcp->dev, "%s(port=%d)\n", __func__, port); mutex_lock(&dcp->hpd_mutex); @@ -2414,6 +2614,23 @@ int dcp_dptx_connect_oob(struct platform_device *pdev, u32 port) struct apple_dcp *dcp = platform_get_drvdata(pdev); int ret; + if (dcp_is_external_h17p(dcp)) { + cancel_delayed_work_sync(&dcp->typec_reconnect_wq); + cancel_delayed_work_sync(&dcp->placeholder_edid_wq); + cancel_work_sync(&dcp->external_stop_work); + guard(mutex)(&dcp->external_lock); + if (!dcp->external_cable) + dcp->external_generation++; + dcp->external_cable = true; + dcp_external_publish_generation(dcp); + scoped_guard(mutex, &dcp->hpd_mutex) { + WRITE_ONCE(dcp->typec_cable_connected, true); + dcp->typec_generation++; + dcp->placeholder_retried = false; + } + return dcp_external_link_locked(dcp); + } + if (dcp_is_typec_output(dcp)) { cancel_delayed_work_sync(&dcp->placeholder_edid_wq); guard(mutex)(&dcp->hpd_mutex); @@ -2436,6 +2653,9 @@ int dcp_dptx_disconnect_oob(struct platform_device *pdev, u32 port) { struct apple_dcp *dcp = platform_get_drvdata(pdev); + if (dcp_is_external_h17p(dcp)) + return dcp_external_disconnect(dcp, true); + if (dcp_is_typec_output(dcp)) { scoped_guard(mutex, &dcp->hpd_mutex) { WRITE_ONCE(dcp->typec_cable_connected, false); @@ -2740,6 +2960,19 @@ void dcp_poweron(struct platform_device *pdev) unsigned long remaining; int ret; + if (dcp_is_external_h17p(dcp)) { + cancel_delayed_work_sync(&dcp->typec_reconnect_wq); + cancel_delayed_work_sync(&dcp->placeholder_edid_wq); + cancel_work_sync(&dcp->external_stop_work); + guard(mutex)(&dcp->external_lock); + ret = dcp_external_start_locked(dcp); + if (!ret) + dcp->external_crtc_generation = dcp->external_generation; + else + dev_err(dcp->dev, "external display activation failed: %d\n", ret); + return; + } + if (dcp_uses_soft_dpms(dcp)) { if (dcp->fw_compat == DCP_FIRMWARE_H17P && dcp->hw.iomfb_method_profile != DCP_IOMFB_METHODS_H17G && @@ -2814,11 +3047,30 @@ void dcp_poweron(struct platform_device *pdev) av_service_connect(dcp); } +/* Same-CRTC helper dependencies order this token refresh before later disables. */ +void dcp_external_recover(struct platform_device *pdev) +{ + struct apple_dcp *dcp = platform_get_drvdata(pdev); + + if (dcp_is_external_h17p(dcp)) + dcp_poweron(pdev); +} + void dcp_poweroff(struct platform_device *pdev) { struct apple_dcp *dcp = platform_get_drvdata(pdev); int ret; + if (dcp_is_external_h17p(dcp)) { + cancel_delayed_work_sync(&dcp->typec_reconnect_wq); + cancel_delayed_work_sync(&dcp->placeholder_edid_wq); + guard(mutex)(&dcp->external_lock); + ret = dcp_external_stop_locked(dcp, dcp->external_crtc_generation); + if (ret) + dev_err(dcp->dev, "external display stop failed: %d\n", ret); + return; + } + cancel_delayed_work(&dcp->swap_watchdog_wq); /* Internal H17P DPMS is a brightness present, never a pipe stop. */ @@ -3368,6 +3620,7 @@ static void dcp_disable_typec_work(struct apple_dcp *dcp, bool release_cable) /* Block new enqueues as well as draining users of the AFK endpoints. */ disable_delayed_work_sync(&dcp->typec_reconnect_wq); disable_delayed_work_sync(&dcp->placeholder_edid_wq); + cancel_work_sync(&dcp->external_stop_work); } static void dcp_enable_typec_work(struct apple_dcp *dcp) @@ -3603,7 +3856,9 @@ static void dcp_comp_unbind(struct device *dev, struct device *main, void *data) if (dcp->retain_dma) { /* No firmware stop is acknowledged: detach host users, never DMA. */ - WRITE_ONCE(dcp->quiescing, true); + scoped_guard(mutex, &dcp->iomfb.lock) + WRITE_ONCE(dcp->quiescing, true); + cancel_work_sync(&dcp->external_stop_work); if (dcp->crtc && !dcp->drm_retained) { drm_dev_get(dcp->crtc->base.dev); dcp->drm_retained = true; @@ -3749,6 +4004,8 @@ static int dcp_platform_probe(struct platform_device *pdev) * registered below, before the DRM device binds. */ mutex_init(&dcp->hpd_mutex); + mutex_init(&dcp->external_lock); + INIT_WORK(&dcp->external_stop_work, dcp_external_stop_work); mutex_init(&dcp->tb_lock); spin_lock_init(&dcp->mode_state.lock); spin_lock_init(&dcp->dcpavserv.lock); @@ -3761,6 +4018,7 @@ static int dcp_platform_probe(struct platform_device *pdev) experimental_h17p_external_bw_request)) return dev_err_probe(dev, -EINVAL, "External bandwidth offsets are unset or invalid\n"); + iomfb_pause_presentation(dcp, true); dcp->hw.firmware_scratch = experimental_h17p_external_bw_scratch; dcp->hw.firmware_request = experimental_h17p_external_bw_request; } @@ -4026,6 +4284,19 @@ static int dcp_platform_suspend(struct device *dev) { struct apple_dcp *dcp = dev_get_drvdata(dev); + if (dcp_is_external_h17p(dcp)) { + int ret; + + dcp_disable_typec_work(dcp, false); + ret = dcp_external_disconnect(dcp, false); + if (ret) { + dcp_enable_typec_work(dcp); + return ret; + } + device_set_wakeup_path(dev); + return 0; + } + dcp_drain_for_sleep(dcp); /* * The Type-C route reports cable removal through diff --git a/drivers/gpu/drm/apple/dcp.h b/drivers/gpu/drm/apple/dcp.h index 600899c71546b..c2b02a5309eab 100644 --- a/drivers/gpu/drm/apple/dcp.h +++ b/drivers/gpu/drm/apple/dcp.h @@ -34,6 +34,7 @@ struct apple_encoder { void dcp_poweroff(struct platform_device *pdev); void dcp_poweron(struct platform_device *pdev); +void dcp_external_recover(struct platform_device *pdev); int dcp_set_crc(struct drm_crtc *crtc, bool enabled); int dcp_crtc_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state); int dcp_get_connector_type(struct platform_device *pdev); diff --git a/drivers/gpu/drm/apple/iomfb.c b/drivers/gpu/drm/apple/iomfb.c index 786eb53383329..cc0989199b714 100644 --- a/drivers/gpu/drm/apple/iomfb.c +++ b/drivers/gpu/drm/apple/iomfb.c @@ -430,6 +430,45 @@ bool iomfb_queue_drain(struct apple_dcp *dcp, unsigned long timeout) } } +/* Admission is independent of transport: replies and control RPCs remain live. */ +void iomfb_pause_presentation(struct apple_dcp *dcp, bool pause) +{ + guard(mutex)(&dcp->iomfb.lock); + dcp->iomfb.presentation_paused = pause; + if (!pause && dcp->external_hotplug_pending) { + unsigned int action; + + dcp->external_hotplug_pending = false; + action = dcp_mode_hotplug(&dcp->mode_state, dcp->external_hotplug_connected, + dcp->connector ? &dcp->connector->connected : NULL); + dcp_handle_hotplug_actions(dcp, action); + } +} + +/* A dead transport is not proof that DMA or a pending present has stopped. */ +int iomfb_queue_settle(struct apple_dcp *dcp, unsigned long timeout) +{ + unsigned long deadline = jiffies + timeout; + bool idle; + + for (;;) { + mutex_lock(&dcp->iomfb.lock); + if (READ_ONCE(dcp->crashed) || dcp->iomfb.stopped) { + mutex_unlock(&dcp->iomfb.lock); + return -EIO; + } + idle = !dcp->iomfb.active && !dcp->iomfb.queued && + list_empty(&dcp->iomfb.pending) && + !dcp->present_state_h17p.pending && iomfb_channels_idle(dcp); + mutex_unlock(&dcp->iomfb.lock); + if (idle) + return 0; + if (time_after_eq(jiffies, deadline)) + return -ETIMEDOUT; + usleep_range(2000, 4000); + } +} + int iomfb_queue(struct apple_dcp *dcp, struct iomfb_transaction *transaction) { int ret = 0; @@ -437,6 +476,8 @@ int iomfb_queue(struct apple_dcp *dcp, struct iomfb_transaction *transaction) mutex_lock(&dcp->iomfb.lock); if (READ_ONCE(dcp->crashed) || dcp->iomfb.stopped) { ret = -EIO; + } else if (transaction->presentation && dcp->iomfb.presentation_paused) { + ret = -EAGAIN; } else if (transaction->brightness_only && dcp->iomfb.backlight_queued) { ret = -EALREADY; } else if (dcp->iomfb.queued >= 32) { @@ -506,7 +547,7 @@ void iomfb_queue_crc_h17p(struct apple_dcp *dcp, u32 swap_id) lockdep_assert_held(&dcp->iomfb.lock); if (!READ_ONCE(dcp->crc_enabled) || READ_ONCE(dcp->crashed) || - dcp->iomfb.stopped || dcp->iomfb.queued >= 32) + dcp->iomfb.stopped || dcp->iomfb.presentation_paused || dcp->iomfb.queued >= 32) return; crc = kzalloc_obj(*crc); if (!crc) @@ -1119,6 +1160,13 @@ int dcp_crtc_atomic_modeset(struct drm_crtc *crtc, int ret = -EIO; bool modeset; + /* Disconnect cannot pass the admission check before these RPCs finish. */ + guard(mutex)(&dcp->external_lock); + if (dcp->hw.iomfb_method_profile == DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL && + (!dcp->external_running || dcp->external_error || READ_ONCE(dcp->quiescing) || + READ_ONCE(dcp->crashed) || READ_ONCE(dcp->iomfb.presentation_paused))) + return -EPIPE; + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); if (!crtc_state) return 0; @@ -1374,6 +1422,7 @@ static void iomfb_backlight_kick(struct apple_dcp *dcp) transaction->start = iomfb_backlight_start; transaction->release = iomfb_backlight_release; transaction->brightness_only = true; + transaction->presentation = true; if (iomfb_queue(dcp, transaction)) kfree(transaction); } @@ -1434,18 +1483,29 @@ static void iomfb_queue_atomic(struct apple_dcp *dcp, struct drm_crtc *crtc, struct drm_atomic_state *state) { struct iomfb_atomic_transaction *atomic; + int ret; + if (READ_ONCE(dcp->iomfb.presentation_paused)) { + schedule_work(&dcp->vblank_wq); + return; + } atomic = kzalloc_obj(*atomic); if (!atomic) goto failed; atomic->transaction.start = iomfb_atomic_start; + atomic->transaction.presentation = true; atomic->transaction.release = iomfb_atomic_release; atomic->state = drm_atomic_state_get(state); atomic->crtc = crtc; atomic->dcp = dcp; - if (!iomfb_queue(dcp, &atomic->transaction)) + ret = iomfb_queue(dcp, &atomic->transaction); + if (!ret) return; iomfb_atomic_release(&atomic->transaction); + if (ret == -EAGAIN) { + schedule_work(&dcp->vblank_wq); + return; + } failed: WRITE_ONCE(dcp->crashed, true); schedule_work(&dcp->vblank_wq); diff --git a/drivers/gpu/drm/apple/iomfb_h17p.c b/drivers/gpu/drm/apple/iomfb_h17p.c index 49fcabaeb7c66..b3ae896426e25 100644 --- a/drivers/gpu/drm/apple/iomfb_h17p.c +++ b/drivers/gpu/drm/apple/iomfb_h17p.c @@ -718,6 +718,105 @@ bool iomfb_check_callback_h17p(struct apple_dcp *dcp, int tag, u32 in_len, return true; } +static_assert(sizeof(struct dcp_set_power_state_resp) == 8); + +struct dcp_external_power_cookie { + struct dcp_wait_cookie wait; + bool on; +}; + +/* A473's envelope is known; no OFF reply has been qualified as a stop. */ +static void iomfb_external_power_complete(struct apple_dcp *dcp, void *out, void *data) +{ + struct dcp_external_power_cookie *cookie = data; + const struct dcp_packet_header *header; + int ret = -EPROTO; + + if (out) { + header = out - sizeof(struct dcp_set_power_state_req_h17p) - sizeof(*header); + if (header->in_len == sizeof(struct dcp_set_power_state_req_h17p) && + header->out_len == sizeof(struct dcp_set_power_state_resp)) { + u64 reply = get_unaligned_le64(out); + + WRITE_ONCE(dcp->external_power_reply, reply); + WRITE_ONCE(dcp->external_power_reply_valid, true); + dev_info(dcp->dev, "A473 power=%u reply=%#llx%s\n", + cookie->on, reply, cookie->on ? "" : " (stop unqualified)"); + ret = cookie->on ? (reply == 1 ? 0 : -EREMOTEIO) : -EOPNOTSUPP; + } + } + WRITE_ONCE(cookie->wait.status, ret); + complete(&cookie->wait.done); + kref_put(&cookie->wait.refcount, release_wait_cookie); +} + +static bool iomfb_external_reply_size(void *out, u32 in_len, u32 out_len) +{ + const struct dcp_packet_header *header; + + if (!out) + return false; + header = out - in_len - sizeof(*header); + return header->in_len == in_len && header->out_len == out_len; +} + +static void iomfb_external_parameter_complete(struct apple_dcp *dcp, void *out, void *data) +{ + struct dcp_set_power_state_req_h17p request = { .unklong = 1 }; + + if (!iomfb_external_reply_size(out, sizeof(struct dcp_set_parameter_dcp), sizeof(u32))) { + iomfb_external_power_complete(dcp, NULL, data); + return; + } + dcp_set_power_state(dcp, true, &request, iomfb_external_power_complete, data); +} + +static void iomfb_external_device_complete(struct apple_dcp *dcp, void *out, void *data) +{ + struct dcp_set_parameter_dcp parameter = { + .param = IOMFBPARAM_ADAPTIVE_SYNC, + .count = 3, + }; + + if (!iomfb_external_reply_size(out, sizeof(u32), sizeof(u32))) { + iomfb_external_power_complete(dcp, NULL, data); + return; + } + /* Preserve the existing external power-on preparation and payload semantics. */ + dcp_set_parameter_dcp(dcp, false, ¶meter, iomfb_external_parameter_complete, data); +} + +int iomfb_external_power_h17p(struct apple_dcp *dcp, bool on) +{ + struct dcp_set_power_state_req_h17p request = { .unklong = on }; + struct dcp_external_power_cookie *cookie; + int ret; + + if (READ_ONCE(dcp->crashed) || READ_ONCE(dcp->iomfb.stopped)) + return -EIO; + cookie = kzalloc_obj(*cookie); + if (!cookie) + return -ENOMEM; + cookie->on = on; + WRITE_ONCE(dcp->external_power_reply_valid, false); + cookie->wait.status = -ETIMEDOUT; + init_completion(&cookie->wait.done); + kref_init(&cookie->wait.refcount); + kref_get(&cookie->wait.refcount); + if (on) { + u32 handle = 2; + + iomfb_opaque_x_reset_h17p(dcp); + dcp_set_display_device(dcp, false, &handle, iomfb_external_device_complete, cookie); + } else { + dcp_set_power_state(dcp, true, &request, iomfb_external_power_complete, cookie); + } + ret = wait_for_completion_timeout(&cookie->wait.done, msecs_to_jiffies(500)); + ret = ret ? (int)READ_ONCE(cookie->wait.status) : -ETIMEDOUT; + kref_put(&cookie->wait.refcount, release_wait_cookie); + return ret; +} + void iomfb_start_h17p(struct apple_dcp *dcp) { WRITE_ONCE(dcp->pipe_enabled_h17p, false); diff --git a/drivers/gpu/drm/apple/iomfb_template.c b/drivers/gpu/drm/apple/iomfb_template.c index 575b99606e1ac..dd57ffaaafe09 100644 --- a/drivers/gpu/drm/apple/iomfb_template.c +++ b/drivers/gpu/drm/apple/iomfb_template.c @@ -1860,6 +1860,9 @@ static void dcpep_cb_hotplug(struct apple_dcp *dcp, u64 *connected) if (dcp_is_typec_output(dcp) && *connected && dcp->nr_modes) complete_all(&dcp->typec_iomfb_hpd_ready); + if (dcp_external_hotplug(dcp, !!(*connected))) + return; + action = dcp_mode_hotplug(&dcp->mode_state, !!(*connected), connector ? &connector->connected : NULL); /* diff --git a/tools/diagnostics/test-apple-external-lifecycle.py b/tools/diagnostics/test-apple-external-lifecycle.py new file mode 100644 index 0000000000000..35649c07c3412 --- /dev/null +++ b/tools/diagnostics/test-apple-external-lifecycle.py @@ -0,0 +1,529 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only +"""Host-only interleavings through production external lifecycle and queue code.""" +import hashlib +import os +from pathlib import Path +import shlex +import subprocess +import tempfile + +root = Path(__file__).resolve().parents[2] +base = root / 'drivers/gpu/drm/apple' +dcp = (base / 'dcp.c').read_text() +iomfb = (base / 'iomfb.c').read_text() +h17p = (base / 'iomfb_h17p.c').read_text() +drm = (base / 'apple_drv.c').read_text() + + +def block(source, marker): + start = source.index(marker) + begin = source.index('{', start) + end, depth = begin + 1, 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[start:end] + + +def function(source, name): + import re + marker = re.search(r'^(?:static )?(?:bool|void|int) ' + name + r'\([^;{]*\)\n\{', source, re.M) + assert marker, name + return block(source, marker.group()[:-1]) + + +def external_branch(name, result): + body = function(dcp, name) + branch = block(body, 'if (dcp_is_external_h17p(dcp)) {') + return body[:body.index('if (dcp_is_external_h17p(dcp)) {')] + branch + result + '\n}' + + +parts = [function(iomfb, 'dcp_handle_hotplug_actions')] +for name in ('iomfb_channels_idle', 'iomfb_pause_presentation', 'iomfb_queue_settle', 'iomfb_queue'): + parts.append(function(iomfb, name)) +parts.append(block(h17p, 'struct dcp_external_power_cookie {') + ';') +for name in ('iomfb_external_power_complete', 'iomfb_external_reply_size', + 'iomfb_external_parameter_complete', 'iomfb_external_device_complete'): + parts.append(function(h17p, name)) +parts.append(function(h17p, 'iomfb_external_power_h17p').replace( + 'int iomfb_external_power_h17p(', 'static int test_wire_power(')) +# Model a future qualified OFF policy only in this harness. The production +# decoder is also tested directly and currently MUST reject every OFF reply. +parts.append(r''' +static int iomfb_external_power_h17p(struct apple_dcp *p, bool on) +{ + int ret = test_wire_power(p, on); + return test_qualified_off && !on && ret == -EOPNOTSUPP && !p->external_power_reply ? 0 : ret; +} +''') +for name in ('dcp_is_external_h17p', 'dcp_external_report_disconnected', + 'dcp_external_publish_generation', 'dcp_external_link_locked', + 'dcp_external_start_locked', 'dcp_external_stop_locked', + 'dcp_external_disconnect', 'dcp_external_stop_work', 'dcp_external_hotplug'): + parts.append(function(dcp, name)) +parts += [external_branch('dcp_dptx_connect_oob', '\nreturn -ENOSYS;'), + external_branch('dcp_poweron', ''), external_branch('dcp_poweroff', ''), + function(dcp, 'dcp_external_recover')] +# Include real admission call sites; only their allocation/start/release dependencies are mocked. +parts += [function(iomfb, 'iomfb_queue_atomic'), function(iomfb, 'iomfb_backlight_kick'), + function(iomfb, 'iomfb_queue_crc_h17p')] +parts += [function(iomfb, 'dcp_crtc_atomic_modeset')] +parts += [function(drm, 'apple_crtc_atomic_enable'), function(drm, 'apple_crtc_atomic_disable')] +route_admission = function(dcp, 'dcp_typec_route_deactivate') +route_admission = route_admission[:route_admission.index('\n\t/*')] + '\n\treturn 0;\n}' +route_admission = route_admission.replace('dcp_typec_route_deactivate(', 'test_route_deactivate_admission(') +parts.append(route_admission) +production = '\n'.join(parts) +print('Production lifecycle SHA256:', hashlib.sha256(production.encode()).hexdigest()) +prefix = r''' +#include +#include +#include +#include +#include +#include +#include +#include +typedef uint32_t u32; +typedef uint64_t u64; +#define READ_ONCE(x) (x) +#define WRITE_ONCE(x, v) ((x) = (v)) +#define msecs_to_jiffies(x) (x) +#define time_after_eq(a, b) ((a) >= (b)) +#define DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL 2 +#define dev_err(...) ((void)0) +#define dev_info(...) ((void)0) +#define WARN_ONCE(...) ((void)0) +enum { DCP_FIRMWARE_V_12_3, DCP_FIRMWARE_V_13_5, DCP_FIRMWARE_H17P }; +#define IOMFBPARAM_ADAPTIVE_SYNC 1 +struct dcp_set_parameter_dcp { u32 param, value[8], count; }; +struct mutex { bool held; }; +static void mutex_lock(struct mutex *m) { assert(!m->held); m->held = true; } +static void mutex_unlock(struct mutex *m) { assert(m->held); m->held = false; } +static struct mutex *acquire(struct mutex *m) { mutex_lock(m); return m; } +static void release(struct mutex **m) { if (*m) mutex_unlock(*m); } +static struct mutex *once(struct mutex *m) { mutex_unlock(m); return NULL; } +#define JOIN2(a,b) a##b +#define JOIN(a,b) JOIN2(a,b) +#define guard(kind) struct mutex *JOIN(guard_, __COUNTER__) __attribute__((cleanup(release))) = acquire +#define scoped_guard(kind, value) for (struct mutex *g __attribute__((cleanup(release))) = acquire(value); g; g = once(g)) +#define lockdep_assert_held(m) assert((m)->held) +#define container_of(p, type, member) ((type *)((char *)(p) - offsetof(type, member))) +struct list_head { unsigned int count; }; +static bool list_empty(struct list_head *h) { return !h->count; } +static void list_add_tail(struct list_head *n, struct list_head *h) { (void)n; h->count++; } +#define list_add list_add_tail +struct work_struct { unsigned int scheduled; }; +struct delayed_work { unsigned int cancelled; }; +struct completion { bool done; }; +struct kref { unsigned int count; }; +struct dcp_wait_cookie { struct kref refcount; struct completion done; u32 status; }; +struct dcp_packet_header { char tag[4]; u32 in_len, out_len; }; +struct dcp_set_power_state_req_h17p { u64 unklong; u32 unkint; unsigned char null, padding[3]; } __attribute__((packed)); +struct dcp_set_power_state_resp { u32 unknown, ret; } __attribute__((packed)); +struct device { int unused; }; +struct apple_connector { bool connected; struct work_struct hotplug_wq; }; +struct dptx_port { bool connected; void *service; }; +struct iomfb_transaction { struct list_head link; void (*start)(void *,void *), (*release)(void *); bool brightness_only, presentation; }; +struct platform_device; +struct drm_crtc_state { bool active, active_changed, mode_changed, color_mgmt_changed; void *event; struct { int hdisplay, vdisplay; } mode; }; +struct drm_device { int event_lock; }; +struct drm_crtc { struct drm_crtc_state *state; struct drm_device *dev; }; +struct drm_atomic_state { struct drm_crtc_state *next; }; +struct apple_crtc { struct drm_crtc base; struct platform_device *dcp; }; +#define to_apple_crtc(p) container_of(p, struct apple_crtc, base) +static struct drm_crtc_state *drm_atomic_get_new_crtc_state(struct drm_atomic_state *s, struct drm_crtc *c) { (void)c; return s->next; } +static bool drm_atomic_crtc_needs_modeset(struct drm_crtc_state *s) { return s->mode_changed; } +static unsigned int modesets, vblanks; +static int dcp_crtc_atomic_modeset(struct drm_crtc *, struct drm_atomic_state *); +#define spin_lock_irq(lock) ((void)(lock)) +#define spin_unlock_irq(lock) ((void)(lock)) +static void drm_crtc_send_vblank_event(struct drm_crtc *c, void *e) { (void)c; assert(e); vblanks++; } +struct apple_dcp; +struct iomfb_atomic_transaction { struct iomfb_transaction transaction; struct drm_crtc *crtc; struct drm_atomic_state *state; struct apple_dcp *dcp; }; +struct iomfb_crc_transaction { struct iomfb_transaction transaction; u32 swap_id; }; +struct apple_dcp { + struct device *dev; + struct { int iomfb_method_profile; } hw; + struct mutex external_lock, hpd_mutex; + u64 external_generation, external_crtc_generation, typec_generation, external_rx_generation, external_stop_generation, external_power_reply; + bool external_power_reply_valid, external_power_attempted, quiescing; + int fw_compat; + bool external_prepared, external_cable, external_running, external_stopped; + bool external_hotplug_pending, external_hotplug_connected; + int external_error; + bool crashed, pipe_enabled_h17p, typec_crtc_off, typec_cable_connected, placeholder_retried, crc_enabled; + struct delayed_work typec_reconnect_wq, placeholder_edid_wq; + struct work_struct external_stop_work, vblank_wq; + struct apple_connector *connector, *typec_connector; + struct dptx_port dptxport[1]; + void *avep; + struct { bool pending; } present_state_h17p; + struct { unsigned int depth; } ch_cmd, ch_cb, ch_oobcmd, ch_oobcb, ch_async, ch_oobasync; + struct { + struct mutex lock; + struct list_head pending; + struct iomfb_transaction *active; + unsigned int queued; + bool stopped, presentation_paused, backlight_queued; + struct work_struct work; + } iomfb; + struct { bool valid; } mode_state; + unsigned int retained_dma, retained_fb; +}; +struct platform_device { struct apple_dcp *data; }; +struct mux_control { int unused; }; +struct apple_dcp_typec_route { struct apple_dcp *dcp; struct mux_control *active_xbar; bool tunnel; }; +#define platform_get_drvdata(p) ((p)->data) +static struct apple_dcp *current_dcp; +static unsigned long jiffies; +static unsigned int sleeps, power0, power1, drops, releases, connects, allocations; +static int connect_error, hpd_error, release_error, wire_error; +static bool drain_callback, timeout_reply, absent_reply, test_qualified_off; +static int prep_error; +static bool on_zero_reply; +static u32 wire_in, wire_out; +static char events[256]; +static unsigned int event_count; +static void event(char c) { assert(event_count < sizeof(events) - 1); events[event_count++] = c; } +static bool dcp_external_hotplug(struct apple_dcp *, bool); +static void usleep_range(unsigned int a, unsigned int b) +{ + (void)a; (void)b; + assert(!current_dcp->iomfb.lock.held); + jiffies += 2; sleeps++; + if (drain_callback) { + mutex_lock(¤t_dcp->iomfb.lock); + assert(dcp_external_hotplug(current_dcp, false)); + current_dcp->iomfb.active = NULL; + current_dcp->iomfb.pending.count = current_dcp->iomfb.queued = 0; + current_dcp->present_state_h17p.pending = false; + current_dcp->ch_cb.depth = 0; + mutex_unlock(¤t_dcp->iomfb.lock); + drain_callback = false; + } +} +static unsigned int dcp_mode_hotplug(void *state, bool connected, bool *result) +{ (void)state; if (result) *result = connected; return 1; } +#define DCP_HOTPLUG_NOTIFY 1 +#define DCP_HOTPLUG_VBLANK 2 +static void dcp_handle_hotplug_actions(struct apple_dcp *, unsigned int); +static void schedule_work(struct work_struct *); +static void schedule_work(struct work_struct *w) +{ + w->scheduled++; + if (current_dcp && w == ¤t_dcp->connector->hotplug_wq) event('N'); +} +static void cancel_delayed_work_sync(struct delayed_work *w) +{ assert(!current_dcp->external_lock.held); w->cancelled++; } +static void cancel_work_sync(struct work_struct *w) +{ assert(!current_dcp->external_lock.held); w->scheduled = 0; } +static bool dptx_service_available(struct dptx_port *p) { return p->service != NULL; } +static int dptxport_set_hpd(void *s, bool hpd) +{ (void)s; assert(!hpd); event('H'); drops++; return hpd_error; } +static int dptxport_release_display(void *s) +{ (void)s; event('R'); releases++; return release_error; } + +static void av_service_disconnect(struct apple_dcp *p) { (void)p; } +static int dcp_dptx_connect_raw(struct apple_dcp *p, u32 port) +{ + assert(!port && p->external_lock.held); + if (p->dptxport[0].connected) return 0; + event('C'); connects++; + if (connect_error) return connect_error; + p->dptxport[0].connected = true; + mutex_lock(&p->iomfb.lock); + assert(dcp_external_hotplug(p, true)); + mutex_unlock(&p->iomfb.lock); + return 0; +} +static void kref_init(struct kref *r) { r->count = 1; } +static void kref_get(struct kref *r) { r->count++; } +static void kref_put(struct kref *r, void (*fn)(struct kref *)) { assert(r->count); if (!--r->count) fn(r); } +static void release_wait_cookie(struct kref *r) { allocations--; free(container_of(r, struct dcp_wait_cookie, refcount)); } +static void *allocate(size_t size) { allocations++; return calloc(1, size); } +#define kzalloc_obj(obj) allocate(sizeof(obj)) +static void kfree(void *p) { if (p) { allocations--; free(p); } } +static void init_completion(struct completion *c) { c->done = false; } +static void complete(struct completion *c) { c->done = true; } +static unsigned long wait_for_completion_timeout(struct completion *c, unsigned long timeout) +{ (void)timeout; assert(!current_dcp->iomfb.lock.held); return c->done; } +static u64 get_unaligned_le64(const void *p) { u64 n; memcpy(&n, p, sizeof(n)); return n; } +static struct dcp_wait_cookie *late_cookie; +static void iomfb_external_power_complete(struct apple_dcp *, void *, void *); +static void dcp_set_power_state(struct apple_dcp *p, bool oob, + struct dcp_set_power_state_req_h17p *req, + void (*cb)(struct apple_dcp *, void *, void *), void *cookie) +{ + struct { struct dcp_packet_header header; struct dcp_set_power_state_req_h17p req; struct dcp_set_power_state_resp resp; } packet = {0}; + assert(oob && p->iomfb.presentation_paused && !p->iomfb.lock.held); + assert(req->unklong <= 1 && !req->unkint && !req->null); + event(req->unklong ? '1' : '0'); + if (req->unklong) power1++; else power0++; + packet.header.in_len = wire_in; + packet.header.out_len = wire_out; + packet.resp.unknown = req->unklong && !on_zero_reply ? 1 : 0; + packet.resp.ret = wire_error; + if (timeout_reply) { late_cookie = cookie; return; } + mutex_lock(&p->iomfb.lock); + assert(dcp_external_hotplug(p, req->unklong)); + cb(p, absent_reply ? NULL : &packet.resp, cookie); + mutex_unlock(&p->iomfb.lock); +} +static void iomfb_opaque_x_reset_h17p(struct apple_dcp *p) { (void)p; } +static void dcp_set_display_device(struct apple_dcp *p, bool oob, u32 *handle, + void (*cb)(struct apple_dcp *, void *, void *), void *cookie) +{ + struct { struct dcp_packet_header h; u32 input, output; } packet = {0}; + assert(!oob && *handle == 2); event('D'); + packet.h.in_len = 4; packet.h.out_len = prep_error == 1 ? 0 : 4; + cb(p, &packet.output, cookie); +} +static void dcp_set_parameter_dcp(struct apple_dcp *p, bool oob, struct dcp_set_parameter_dcp *param, + void (*cb)(struct apple_dcp *, void *, void *), void *cookie) +{ + struct { struct dcp_packet_header h; struct dcp_set_parameter_dcp input; u32 output; } packet = {0}; + assert(!oob && param->param == IOMFBPARAM_ADAPTIVE_SYNC && param->count == 3); event('A'); + for (unsigned int i = 0; i < 8; i++) assert(!param->value[i]); + packet.h.in_len = 40; packet.h.out_len = prep_error == 2 ? 0 : 4; + cb(p, &packet.output, cookie); +} +static void dcp_mode_begin(void *state) { (void)state; } +static unsigned int dcp_mode_finish(void *state, bool success, bool *connected) +{ (void)state; (void)success; (void)connected; return 0; } +static int iomfb_modeset_h17p(struct apple_dcp *p, struct drm_crtc_state *s) +{ (void)s; assert(p->external_lock.held && p->external_running); modesets++; return 0; } +#define iomfb_modeset_v12_3 iomfb_modeset_h17p +#define iomfb_modeset_v13_3 iomfb_modeset_h17p +static void iomfb_atomic_start(void *a, void *b) { (void)a; (void)b; } +static void iomfb_atomic_release(void *p) { kfree(p); } +static struct drm_atomic_state *drm_atomic_state_get(struct drm_atomic_state *s) { return s; } +static void iomfb_backlight_start(void *a, void *b) { (void)a; (void)b; } +static void iomfb_backlight_release(void *p) { kfree(p); } +static void iomfb_crc_start(void *a, void *b) { (void)a; (void)b; } +static void iomfb_crc_release(void *p) { kfree(p); } +''' +suffix = r''' +static struct apple_connector connector; +static void initial(struct apple_dcp *p) +{ + assert(!allocations); + memset(p, 0, sizeof(*p)); + memset(&connector, 0, sizeof(connector)); connector.connected = true; + p->connector = &connector; + p->external_prepared = p->external_cable = p->external_running = p->external_power_attempted = true; + p->fw_compat = DCP_FIRMWARE_H17P; + p->external_generation = p->external_crtc_generation = p->external_rx_generation = 1; + p->hw.iomfb_method_profile = DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL; + p->dptxport[0] = (struct dptx_port){ .connected = true, .service = p }; + p->retained_dma = 7; p->retained_fb = 11; + current_dcp = p; + jiffies = sleeps = power0 = power1 = drops = releases = connects = event_count = 0; + connect_error = hpd_error = release_error = wire_error = 0; + drain_callback = timeout_reply = absent_reply = false; + wire_in = 16; wire_out = 8; + late_cookie = NULL; test_qualified_off = true; prep_error = 0; on_zero_reply = false; + memset(events, 0, sizeof(events)); +} +static int stop(struct apple_dcp *p, u64 generation) +{ + guard(mutex)(&p->external_lock); + return dcp_external_stop_locked(p, generation); +} +static void owned(struct apple_dcp *p) +{ assert(p->retained_dma == 7 && p->retained_fb == 11 && !allocations); } +int main(void) +{ + struct apple_dcp p; + struct platform_device dev = { &p }; + initial(&p); + assert(stop(&p, 1) == 0); + assert(!strcmp(events, "0HR") && power0 == 1 && releases == 1); + assert(p.iomfb.presentation_paused && p.external_stopped && !p.crashed); + assert(stop(&p, 1) == 0 && power0 == 1 && releases == 1); + owned(&p); + + /* Atomic, backlight and CRC are rejected; control transactions remain admissible. */ + iomfb_queue_atomic(&p, NULL, NULL); + iomfb_backlight_kick(&p); + p.crc_enabled = true; + mutex_lock(&p.iomfb.lock); iomfb_queue_crc_h17p(&p, 1); mutex_unlock(&p.iomfb.lock); + assert(!p.crashed && !p.iomfb.queued && !allocations); + struct iomfb_transaction presentation = { .presentation = true }; + struct iomfb_transaction control = {0}; + assert(iomfb_queue(&p, &presentation) == -EAGAIN); + assert(iomfb_queue(&p, &control) == 0); + p.iomfb.queued = p.iomfb.pending.count = 0; + + /* A callback must be able to acquire iomfb.lock and complete a drain. */ + initial(&p); + p.iomfb.active = &presentation; p.iomfb.pending.count = p.iomfb.queued = 1; + p.present_state_h17p.pending = true; p.ch_cb.depth = 1; drain_callback = true; + assert(stop(&p, 1) == 0 && sleeps && power0 == 1 && !p.crashed); + owned(&p); + + /* Every channel, queued-count, active, pending and present state blocks strict drain. */ + for (unsigned int field = 0; field < 10; field++) { + initial(&p); + if (field == 0) p.ch_cmd.depth = 1; + if (field == 1) p.ch_cb.depth = 1; + if (field == 2) p.ch_oobcmd.depth = 1; + if (field == 3) p.ch_oobcb.depth = 1; + if (field == 4) p.ch_async.depth = 1; + if (field == 5) p.ch_oobasync.depth = 1; + if (field == 6) p.iomfb.active = &presentation; + if (field == 7) p.iomfb.pending.count = 1; + if (field == 8) p.present_state_h17p.pending = true; + if (field == 9) p.iomfb.queued = 1; + assert(stop(&p, 1) == -ETIMEDOUT && !power0 && !drops && !releases); + assert(p.iomfb.presentation_paused && p.external_error == -ETIMEDOUT); + assert(stop(&p, 1) == -ETIMEDOUT && !power0); + dcp_external_recover(&dev); + assert(!power1 && !p.crashed); + owned(&p); + } + for (unsigned int fault = 0; fault < 7; fault++) { + initial(&p); + if (fault == 0) p.crashed = true; + if (fault == 1) p.iomfb.stopped = true; + if (fault == 2) wire_out = 12; + if (fault == 3) wire_in = 12; + if (fault == 4) { wire_error = 5; test_qualified_off = false; } + if (fault == 5) hpd_error = -EIO; + if (fault == 6) release_error = -EIO; + assert(stop(&p, 1) < 0 && p.external_error && p.iomfb.presentation_paused); + assert(p.dptxport[0].connected && !p.external_stopped); + assert(power0 == (fault > 1)); + assert(releases == (fault == 6)); + owned(&p); + } + initial(&p); timeout_reply = true; + assert(stop(&p, 1) == -ETIMEDOUT && late_cookie && allocations == 1); + assert(!drops && !releases && p.dptxport[0].connected); + iomfb_external_power_complete(&p, NULL, late_cookie); /* late callback owns its reference */ + owned(&p); + + /* Physical removal + new cable before old CRTC disable: stale token cannot stop new link. */ + initial(&p); + assert(dcp_external_disconnect(&p, true) == 0); + assert(!p.external_cable && !connector.connected && p.external_generation == 2); + assert(dcp_dptx_connect_oob(&dev, 0) == 0); + assert(p.external_generation == 3 && !p.external_running && !connector.connected); + assert(p.iomfb.presentation_paused && power1 == 0); + dcp_poweroff(&dev); + assert(power0 == 1 && !p.external_running); /* token still belongs to generation 1 */ + /* A stale disable must still deliver host DRM event bookkeeping. */ + struct drm_crtc_state inactive = { .active_changed = true, .event = &p }; + struct drm_device drm_dev = {0}; + struct apple_crtc crtc = { .dcp = &dev, .base = { .state = &inactive, .dev = &drm_dev } }; + struct drm_atomic_state atomic = { .next = &inactive }; + apple_crtc_atomic_disable(&crtc.base, &atomic); + assert(power0 == 1 && !inactive.event && vblanks == 1); + /* Active -> active recovery invokes the real CRTC hook and refreshes the token. */ + struct drm_crtc_state active = { .active = true, .mode_changed = true, .mode = { 1920, 1080 } }; + atomic.next = &active; + apple_crtc_atomic_enable(&crtc.base, &atomic); + assert(modesets == 1); + assert(p.external_crtc_generation == 3 && power1 == 1); + dcp_poweroff(&dev); + assert(power0 == 2 && p.external_stopped); + dcp_poweroff(&dev); assert(power0 == 2); + owned(&p); + + /* Uncertain physical stop reports cable loss but keeps the resource and blocks restart. */ + initial(&p); test_qualified_off = false; + assert(dcp_external_disconnect(&p, true) == -EOPNOTSUPP); + assert(!connector.connected && p.dptxport[0].connected && !releases); + assert(dcp_dptx_connect_oob(&dev, 0) == -EOPNOTSUPP && !power1); + assert(p.external_crtc_generation == 1 && p.iomfb.presentation_paused); + owned(&p); + /* A deferred old RX disconnect cannot stop a later connection epoch. */ + initial(&p); + mutex_lock(&p.iomfb.lock); + assert(dcp_external_hotplug(&p, false)); + mutex_unlock(&p.iomfb.lock); + assert(p.external_stop_generation == 1); + p.external_generation = p.external_rx_generation = p.external_crtc_generation = 3; + dcp_external_stop_work(&p.external_stop_work); + assert(!power0 && !drops && p.external_running); + owned(&p); + + /* Before initialization there is no firmware/shared-memory transport to touch. */ + initial(&p); + p.external_prepared = p.external_power_attempted = p.external_running = false; + p.dptxport[0].connected = false; + assert(stop(&p, 1) == 0 && !power0 && !power1 && !releases); + assert(dcp_dptx_connect_oob(&dev, 0) == -EAGAIN && !connects); + owned(&p); + + /* Link hookup alone never powers the pipe. A first activation preserves DA1 order. */ + initial(&p); + p.external_power_attempted = p.external_running = false; + p.iomfb.presentation_paused = true; + p.dptxport[0].connected = false; + assert(dcp_dptx_connect_oob(&dev, 0) == 0 && connects == 1 && !power1); + dcp_external_recover(&dev); + assert(strstr(events, "DA1") && p.external_running && power1 == 1); + owned(&p); + + /* Exact ON requires the 64-bit reply 1; all-zero must not activate modesets. */ + initial(&p); + p.external_running = false; p.external_stopped = true; + p.iomfb.presentation_paused = true; on_zero_reply = true; + unsigned int previous_modesets = modesets; + dcp_external_recover(&dev); + assert(p.external_error == -EREMOTEIO && !p.external_running); + assert(p.external_power_reply_valid && p.external_power_reply == 0); + assert(dcp_crtc_atomic_modeset(&crtc.base, &atomic) == -EPIPE); + apple_crtc_atomic_enable(&crtc.base, &atomic); + assert(modesets == previous_modesets && power1 == 1); + owned(&p); + + /* Production OFF policy always records but refuses to qualify its 8-byte reply. */ + initial(&p); p.iomfb.presentation_paused = true; wire_error = 0x1234; + assert(test_wire_power(&p, false) == -EOPNOTSUPP); + assert(p.external_power_reply_valid && p.external_power_reply == (0x1234ULL << 32)); + assert(!drops && !releases); + owned(&p); + /* Notification delivery is deferred: a client can reenter the lifecycle lock. */ + initial(&p); + assert(dcp_external_disconnect(&p, true) == 0); + assert(p.connector->hotplug_wq.scheduled && !p.external_lock.held); + mutex_lock(&p.external_lock); mutex_unlock(&p.external_lock); + + /* Fabric cleanup cannot release a failed or still-running external route. */ + initial(&p); + struct apple_dcp_typec_route route = { .dcp = &p }; + assert(test_route_deactivate_admission(&route) == -EBUSY); + p.external_error = -EOPNOTSUPP; + assert(test_route_deactivate_admission(&route) == -EOPNOTSUPP); + p.external_error = 0; p.external_stopped = true; + assert(test_route_deactivate_admission(&route) == 0); + owned(&p); + + /* Missing replies and malformed first-start preparation fail closed. */ + initial(&p); p.iomfb.presentation_paused = true; absent_reply = true; + assert(test_wire_power(&p, false) == -EPROTO && !p.external_power_reply_valid); + owned(&p); + for (int stage = 1; stage <= 2; stage++) { + initial(&p); p.external_running = false; p.iomfb.presentation_paused = true; + prep_error = stage; + dcp_external_recover(&dev); + assert(p.external_error == -EPROTO && !power1 && p.iomfb.presentation_paused); + owned(&p); + } + puts("PASS: strict drain, callbacks, admission, exact RPC, stop/restart, stale epochs, modeset gate, retained ownership"); + return 0; +} +''' +with tempfile.TemporaryDirectory(prefix='apple-external-lifecycle-') as tmp: + tmp = Path(tmp) + unit = tmp / 'test.c' + unit.write_text(prefix + production + suffix) + command = shlex.split(os.environ.get('CC', 'clang')) + command += ['-std=gnu11', '-Wall', '-Wextra', '-Werror', + '-Wno-unused-variable', '-Wno-unused-parameter', '-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) From 22d427d629caba743a74648e13699655b0a1a94c Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Mon, 12 Oct 2026 20:13:34 +1100 Subject: [PATCH 2/4] drm/apple: retry initial external DPTX service discovery Distinguish a service not yet published from terminal session closure before issuing firmware commands. Keep initial publication retryable, preserve sticky failures after real connection commands, and schedule asynchronous reconnect when the service arrives late. The R28 hardware trial exposed the startup ordering regression. Extracted production tests cover initial and partial publication, closure, late arrival, and retained firmware failures. Generated-by: LLM Assisted-by: LLM Signed-off-by: Ryan Murray --- MAINTAINERS | 1 + drivers/gpu/drm/apple/dcp.c | 16 +- drivers/gpu/drm/apple/dptx-service.h | 4 +- drivers/gpu/drm/apple/dptxep.c | 8 + drivers/gpu/drm/apple/dptxep.h | 2 + tools/diagnostics/test-apple-afk-admission.py | 2 +- .../diagnostics/test-apple-dptx-readiness.py | 241 ++++++++++++++++++ .../test-apple-external-lifecycle.py | 3 +- 8 files changed, 270 insertions(+), 7 deletions(-) create mode 100644 tools/diagnostics/test-apple-dptx-readiness.py diff --git a/MAINTAINERS b/MAINTAINERS index 508097a2e7db3..fede536d3e31e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2021,6 +2021,7 @@ F: tools/diagnostics/apple-j700-dp-test.c F: tools/diagnostics/test-apple-afk-admission.py F: tools/diagnostics/test-apple-atc-unwind.py F: tools/diagnostics/test-apple-dcp-readiness.py +F: tools/diagnostics/test-apple-dptx-readiness.py F: tools/diagnostics/test-apple-external-lifecycle.py F: tools/diagnostics/test-apple-j700-crossbar.py F: tools/diagnostics/test-apple-j700-d134.py diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index da176a910f46b..8da531f342478 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -2188,11 +2188,13 @@ bool dcp_has_typec_routes(struct platform_device *pdev) #define DPTX_RECONNECT_DELAY msecs_to_jiffies(1000) #define DPTX_RECONNECT_RETRIES 5 -static int dcp_dptx_connect_raw(struct apple_dcp *dcp, u32 port) +static int dcp_dptx_connect_raw(struct apple_dcp *dcp, u32 port, bool *waiting_service) { unsigned long timeout; int ret = 0; + if (waiting_service) + *waiting_service = false; if (!dcp->phy) { dev_warn(dcp->dev, "dcp_dptx_connect: missing phy\n"); return -ENODEV; @@ -2208,6 +2210,11 @@ static int dcp_dptx_connect_raw(struct apple_dcp *dcp, u32 port) if (!dptx_service_available(&dcp->dptxport[port])) { dev_warn(dcp->dev, "dcp_dptx_connect: dptx service for port %d not enabled\n", port); ret = -ENODEV; + /* No firmware request has been made; the first publication can retry. */ + if (waiting_service && !READ_ONCE(dcp->dptxport[port].closed)) { + *waiting_service = true; + ret = -EAGAIN; + } goto out_unlock; } @@ -2318,6 +2325,7 @@ static void dcp_external_publish_generation(struct apple_dcp *dcp) /* external_lock is never taken by the serialized IOMFB receive callbacks. */ static int dcp_external_link_locked(struct apple_dcp *dcp) { + bool waiting_service; int ret; lockdep_assert_held(&dcp->external_lock); @@ -2327,8 +2335,8 @@ static int dcp_external_link_locked(struct apple_dcp *dcp) return dcp->external_error; if (!dcp->external_cable) return -ENODEV; - ret = dcp_dptx_connect_raw(dcp, 0); - if (ret) + ret = dcp_dptx_connect_raw(dcp, 0, &waiting_service); + if (ret && !waiting_service) dcp->external_error = ret; return ret; } @@ -2360,7 +2368,7 @@ static int dcp_external_start_locked(struct apple_dcp *dcp) static int dcp_dptx_connect(struct apple_dcp *dcp, u32 port) { if (!dcp_is_external_h17p(dcp)) - return dcp_dptx_connect_raw(dcp, port); + return dcp_dptx_connect_raw(dcp, port, NULL); guard(mutex)(&dcp->external_lock); return dcp_external_link_locked(dcp); } diff --git a/drivers/gpu/drm/apple/dptx-service.h b/drivers/gpu/drm/apple/dptx-service.h index f806e3253999a..3d990a9ca1ea1 100644 --- a/drivers/gpu/drm/apple/dptx-service.h +++ b/drivers/gpu/drm/apple/dptx-service.h @@ -49,7 +49,9 @@ static inline void dptx_service_unbind(struct apple_epic_service *service) struct dptx_port *port = service->cookie; if (port && port->service == service) { - WRITE_ONCE(port->enabled, false); + WRITE_ONCE(port->closed, true); + /* Make terminal state visible to readers of availability. */ + smp_store_release(&port->enabled, false); WRITE_ONCE(port->connected, false); reinit_completion(&port->enable_completion); } diff --git a/drivers/gpu/drm/apple/dptxep.c b/drivers/gpu/drm/apple/dptxep.c index 451820389f91c..e1d9dfb34dca5 100644 --- a/drivers/gpu/drm/apple/dptxep.c +++ b/drivers/gpu/drm/apple/dptxep.c @@ -844,6 +844,14 @@ static void dptxport_init(struct apple_epic_service *service, const char *name, dptx_service_publish(service); complete(&port->enable_completion); + /* Publication may follow the startup wait or the bounded retry window. */ + if (service->ep->dcp->hw.iomfb_method_profile == + DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL && + READ_ONCE(service->ep->dcp->typec_cable_connected) && + !READ_ONCE(service->ep->dcp->quiescing) && + !READ_ONCE(service->ep->dcp->external_error)) + mod_delayed_work(system_freezable_wq, + &service->ep->dcp->typec_reconnect_wq, 0); } static int dptxport_group_call(struct apple_epic_service *service, u16 group, u32 command, diff --git a/drivers/gpu/drm/apple/dptxep.h b/drivers/gpu/drm/apple/dptxep.h index 14530c99567b1..e731604fb6768 100644 --- a/drivers/gpu/drm/apple/dptxep.h +++ b/drivers/gpu/drm/apple/dptxep.h @@ -50,6 +50,8 @@ struct apple_epic_service; struct dptx_port { bool enabled, connected; + /* A published session closed; unlike initial discovery, this is terminal. */ + bool closed; struct completion enable_completion; struct completion linkcfg_completion; u32 unit; diff --git a/tools/diagnostics/test-apple-afk-admission.py b/tools/diagnostics/test-apple-afk-admission.py index fc5669048bdc9..feb21910537dd 100644 --- a/tools/diagnostics/test-apple-afk-admission.py +++ b/tools/diagnostics/test-apple-afk-admission.py @@ -52,7 +52,7 @@ def extract(file, name): struct apple_dcp_afkep *ep; unsigned long cmd_map[1]; }; struct dptx_port { - bool enabled, connected; int enable_completion; u32 unit; + bool enabled, connected, closed; int enable_completion; u32 unit; struct apple_epic_service *service; }; static void reinit_completion(int *p) { *p = 0; } diff --git a/tools/diagnostics/test-apple-dptx-readiness.py b/tools/diagnostics/test-apple-dptx-readiness.py new file mode 100644 index 0000000000000..d86f1505f7cf3 --- /dev/null +++ b/tools/diagnostics/test-apple-dptx-readiness.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only +"""Exercise production DPTX publication, retries and terminal service failures.""" +import hashlib +import os +from pathlib import Path +import re +import shlex +import subprocess +import tempfile + +root = Path(__file__).resolve().parents[2] +base = root / 'drivers/gpu/drm/apple' +dcp = (base / 'dcp.c').read_text() +ep = (base / 'dptxep.c').read_text() +service = (base / 'dptx-service.h').read_text() + + +def function(source, name): + match = re.search(r'^static (?:inline )?(?:bool|int|void) ' + name + r'\([^;{]*\)\n\{', source, re.M) + assert match, name + start, opening = match.start(), match.end() - 1 + end, depth = opening + 1, 1 + while depth: + depth += (source[end] == '{') - (source[end] == '}') + end += 1 + return source[start:end] + + +body = '\n'.join(function(service, n) for n in ( + 'dptx_service_available', 'dptx_service_bind', 'dptx_service_publish', 'dptx_service_unbind')) +body += '\n' + '\n'.join(function(dcp, n) for n in ( + 'dcp_is_external_h17p', 'dcp_dptx_connect_raw', 'dcp_external_link_locked', + 'dcp_dptx_connect', 'dcp_typec_reconnect_work')) +body += '\n' + function(ep, 'dptxport_init') +print('Production readiness SHA256:', hashlib.sha256(body.encode()).hexdigest()) +prefix = r''' +#include +#include +#include +#include +#include +#include +#include +typedef uint32_t u32; +typedef int64_t s64; +#define READ_ONCE(x) (x) +#define WRITE_ONCE(x, v) ((x) = (v)) +#define smp_load_acquire(p) (*(p)) +#define smp_store_release(p, v) (*(p) = (v)) +#define DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL 2 +#define DPTX_CONNECT_TIMEOUT 2000 +#define DPTX_TUNNEL_CONNECT_TIMEOUT 8000 +#define DPTX_RECONNECT_DELAY 1000 +#define DPTX_RECONNECT_RETRIES 5 +#define DRM_MODE_CONNECTOR_DisplayPort 10 +#define dev_warn(...) ((void)0) +#define dev_info(...) ((void)0) +#define dev_dbg(...) ((void)0) +#define dev_err(...) ((void)0) +#define jiffies_to_msecs(x) (x) +#define usleep_range(a,b) ((void)(a), (void)(b)) +#define system_freezable_wq NULL +#define container_of(p,t,m) ((t *)((char *)(p) - offsetof(t,m))) +#define to_delayed_work(p) container_of(p, struct delayed_work, work) +struct mutex { bool held; }; +static void mutex_lock(struct mutex *m) { assert(!m->held); m->held = true; } +static void mutex_unlock(struct mutex *m) { assert(m->held); m->held = false; } +static struct mutex *acquire(struct mutex *m) { mutex_lock(m); return m; } +static void release(struct mutex **m) { if (*m) mutex_unlock(*m); } +#define guard(kind) struct mutex *g __attribute__((cleanup(release))) = acquire +#define lockdep_assert_held(m) assert((m)->held) +struct completion { bool done; }; +struct work_struct { int unused; }; +struct delayed_work { struct work_struct work; unsigned int scheduled, delay; }; +struct apple_dcp; +struct apple_dcp_afkep { struct apple_dcp *dcp; }; +struct apple_epic_service { void *cookie; struct apple_dcp_afkep *ep; }; +struct dptx_port { + struct apple_epic_service *service; + bool enabled, connected, closed; + s64 unit; + void *atcphy; + struct completion enable_completion, linkcfg_completion; +}; +struct apple_dcp { + struct { int iomfb_method_profile; } hw; + bool external_prepared, external_cable, quiescing, typec_cable_connected; + int external_error; + unsigned int typec_reconnect_tries; + struct mutex external_lock, hpd_mutex; + struct delayed_work typec_reconnect_wq; + void *phy, *dev, *active_typec_route, *avep; + unsigned int dptx_die, dptx_phy, dptx_dfp_port; + int connector_type; + struct dptx_port dptxport[1]; +}; +static unsigned int raw_attempts, firmware_calls, release_calls; +static int injected_error; +static unsigned int fail_stage; +static bool dcp_is_typec_output(struct apple_dcp *p) { (void)p; return true; } +static bool dcp_is_usb4_output(struct apple_dcp *p) { (void)p; return false; } +static bool apple_dp_tunnel_t602x(void) { return false; } +static void reinit_completion(struct completion *c) { c->done = false; } +static void complete(struct completion *c) { c->done = true; } +static unsigned long wait_for_completion_timeout(struct completion *c, unsigned long timeout) +{ (void)timeout; return c->done ? 1 : 0; } +static void mod_delayed_work(void *queue, struct delayed_work *w, unsigned int delay) +{ + struct apple_dcp *p = container_of(w, struct apple_dcp, typec_reconnect_wq); + (void)queue; + assert(!p->external_lock.held && !p->hpd_mutex.held); + w->scheduled++; + w->delay = delay; +} +static int stage(unsigned int step) +{ firmware_calls++; return fail_stage == step ? injected_error : 0; } +static int dptxport_validate_connection(struct apple_epic_service *s, u32 a, u32 b, u32 c) +{ (void)s; (void)a; (void)b; (void)c; raw_attempts++; return stage(1); } +static int dptxport_connect(struct apple_epic_service *s, u32 a, u32 b, u32 c, bool typec) +{ (void)s; (void)a; (void)b; (void)c; (void)typec; return stage(2); } +static int dptxport_request_display(struct apple_epic_service *s) +{ + struct dptx_port *p = s->cookie; + int ret = stage(3); + if (!ret) complete(&p->linkcfg_completion); + return ret; +} +static int dptxport_set_hpd(struct apple_epic_service *s, bool on) +{ (void)s; (void)on; return stage(4); } +static int dptxport_set_hpd_timeout(struct apple_epic_service *s, bool on, unsigned int timeout) +{ (void)timeout; return dptxport_set_hpd(s, on); } +static int dptxport_release_display(struct apple_epic_service *s) +{ (void)s; release_calls++; return 0; } +static void av_service_connect(struct apple_dcp *p) { (void)p; } +''' +suffix = r''' +static void initial(struct apple_dcp *p, struct apple_epic_service *s, struct apple_dcp_afkep *ep) +{ + *p = (struct apple_dcp){ + .hw.iomfb_method_profile = DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL, + .external_prepared = true, .external_cable = true, .typec_cable_connected = true, + .phy = p, + }; + *ep = (struct apple_dcp_afkep){ .dcp = p }; + *s = (struct apple_epic_service){ .ep = ep }; + assert(dptx_service_bind(s, &p->dptxport[0], 0) == 0); + raw_attempts = firmware_calls = release_calls = fail_stage = 0; + injected_error = 0; +} +static int link(struct apple_dcp *p) +{ + guard(mutex)(&p->external_lock); + return dcp_external_link_locked(p); +} +static void publish(struct apple_epic_service *s) +{ dptxport_init(s, "dcpdptx-port-epic", "AppleDCPDPTXRemotePort", 0); } +int main(void) +{ + struct apple_dcp p; + struct apple_epic_service service; + struct apple_dcp_afkep ep; + initial(&p, &service, &ep); + /* Reproduce the observed initial call before the first AFK publication. */ + assert(link(&p) == -EAGAIN && !p.external_error); + assert(!firmware_calls && !release_calls && !p.dptxport[0].connected); + /* Even publication later than the bounded retry window must restart work. */ + for (unsigned int i = 0; i < DPTX_RECONNECT_RETRIES; i++) + dcp_typec_reconnect_work(&p.typec_reconnect_wq.work); + assert(p.typec_reconnect_tries == DPTX_RECONNECT_RETRIES && !p.external_error); + unsigned int queued = p.typec_reconnect_wq.scheduled; + publish(&service); + assert(p.dptxport[0].enabled && p.dptxport[0].enable_completion.done); + assert(p.typec_reconnect_wq.scheduled == queued + 1 && !p.typec_reconnect_wq.delay); + assert(!firmware_calls); /* Publication only schedules; never calls firmware inline. */ + dcp_typec_reconnect_work(&p.typec_reconnect_wq.work); + assert(raw_attempts == 1 && p.dptxport[0].connected && !p.external_error); + assert(!p.typec_reconnect_tries); + + /* A reader between owner publication and enabled=true must also retry. */ + initial(&p, &service, &ep); + p.dptxport[0].service = &service; + assert(link(&p) == -EAGAIN && !p.external_error && !firmware_calls); + publish(&service); + assert(link(&p) == 0 && p.dptxport[0].connected); + + /* A closed published session must remain terminal, even before any power request. */ + dptx_service_unbind(&service); + assert(link(&p) == -ENODEV && p.external_error == -ENODEV); + assert(p.dptxport[0].service == &service); + assert(dptx_service_bind(&service, &p.dptxport[0], 0) == -EBUSY); + unsigned int attempts = raw_attempts; + assert(link(&p) == -ENODEV && raw_attempts == attempts); + + /* Neither ENODEV nor EAGAIN from a real operation can masquerade as unpublished. */ + for (unsigned int phase = 1; phase <= 4; phase++) { + for (unsigned int transient = 0; transient < 2; transient++) { + initial(&p, &service, &ep); + publish(&service); + fail_stage = phase; + injected_error = transient ? -EAGAIN : -ENODEV; + assert(link(&p) == injected_error && p.external_error == injected_error); + unsigned int calls = firmware_calls; + fail_stage = 0; + assert(link(&p) == injected_error && firmware_calls == calls); + } + } + /* Missing configuration is terminal; only the first unpublished service is retryable. */ + initial(&p, &service, &ep); + p.phy = NULL; + assert(link(&p) == -ENODEV && p.external_error == -ENODEV && !firmware_calls); + + /* Legacy raw callers keep ENODEV and do not acquire external retry semantics. */ + initial(&p, &service, &ep); + p.hw.iomfb_method_profile = 0; + assert(dcp_dptx_connect(&p, 0) == -ENODEV && !firmware_calls); + publish(&service); + assert(!p.typec_reconnect_wq.scheduled); + + /* Publication cannot revive a removed cable, failed session, or quiescing device. */ + for (unsigned int state = 0; state < 3; state++) { + initial(&p, &service, &ep); + if (state == 0) p.typec_cable_connected = false; + if (state == 1) p.external_error = -EIO; + if (state == 2) p.quiescing = true; + publish(&service); + assert(!p.typec_reconnect_wq.scheduled && !firmware_calls); + } + puts("PASS: delayed publication, exhausted retry recovery, terminal CLOSE/operation errors, async notification"); + return 0; +} +''' +with tempfile.TemporaryDirectory(prefix='apple-dptx-readiness-') as directory: + directory = Path(directory) + unit = directory / 'test.c' + unit.write_text(prefix + body + suffix) + command = shlex.split(os.environ.get('CC', 'clang')) + command += ['-std=gnu11', '-Wall', '-Wextra', '-Werror', '-Wno-unused-parameter', '-O1', '-g', + '-fsanitize=address,undefined', '-fno-omit-frame-pointer'] + subprocess.run(command + [str(unit), '-o', str(directory / 'test')], check=True) + subprocess.run([str(directory / 'test')], check=True) diff --git a/tools/diagnostics/test-apple-external-lifecycle.py b/tools/diagnostics/test-apple-external-lifecycle.py index 35649c07c3412..e0ab24bcd07b7 100644 --- a/tools/diagnostics/test-apple-external-lifecycle.py +++ b/tools/diagnostics/test-apple-external-lifecycle.py @@ -226,8 +226,9 @@ def external_branch(name, result): { (void)s; event('R'); releases++; return release_error; } static void av_service_disconnect(struct apple_dcp *p) { (void)p; } -static int dcp_dptx_connect_raw(struct apple_dcp *p, u32 port) +static int dcp_dptx_connect_raw(struct apple_dcp *p, u32 port, bool *waiting_service) { + if (waiting_service) *waiting_service = false; assert(!port && p->external_lock.held); if (p->dptxport[0].connected) return 0; event('C'); connects++; From dcd9693d6f455bf20b24d0dca24a608e930ac6cc Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Mon, 12 Oct 2026 20:14:49 +1100 Subject: [PATCH 3/4] drm/apple: select compatible SDR color modes for external H17P Choose advertised full-range 8/10-bit SDR RGB modes for the experimental external H17P profile. Prefer sRGB or BT.709 over default RGB, then use firmware scores. Reject timings that only offer incompatible color modes instead of selecting BT.2020 for native SDR scanout. Leave internal and legacy policy unchanged. The working J700 selected BT.2020 RGB while displaying an SDR desktop. Production-parser tests cover 65 valid, malformed, legacy and high-refresh cases. Hardware color appearance still requires qualification. Generated-by: LLM Assisted-by: LLM Signed-off-by: Ryan Murray --- MAINTAINERS | 1 + drivers/gpu/drm/apple/iomfb_template.c | 4 +- drivers/gpu/drm/apple/parser.c | 44 ++++- drivers/gpu/drm/apple/parser.h | 2 +- tools/diagnostics/test-apple-external-sdr.py | 188 +++++++++++++++++++ 5 files changed, 232 insertions(+), 7 deletions(-) create mode 100644 tools/diagnostics/test-apple-external-sdr.py diff --git a/MAINTAINERS b/MAINTAINERS index fede536d3e31e..38e9edaa75fdd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2023,6 +2023,7 @@ F: tools/diagnostics/test-apple-atc-unwind.py F: tools/diagnostics/test-apple-dcp-readiness.py F: tools/diagnostics/test-apple-dptx-readiness.py F: tools/diagnostics/test-apple-external-lifecycle.py +F: tools/diagnostics/test-apple-external-sdr.py F: tools/diagnostics/test-apple-j700-crossbar.py F: tools/diagnostics/test-apple-j700-d134.py F: tools/diagnostics/test-apple-j700-dp.py diff --git a/drivers/gpu/drm/apple/iomfb_template.c b/drivers/gpu/drm/apple/iomfb_template.c index dd57ffaaafe09..3c580c6a6b08e 100644 --- a/drivers/gpu/drm/apple/iomfb_template.c +++ b/drivers/gpu/drm/apple/iomfb_template.c @@ -985,7 +985,9 @@ static bool dcpep_process_chunks(struct apple_dcp *dcp, dcp->width_mm, dcp->height_mm, dcp->notch_height, dcp->fixed_connector_type == - DRM_MODE_CONNECTOR_eDP); + DRM_MODE_CONNECTOR_eDP, + dcp->hw.iomfb_method_profile == + DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL); if (IS_ERR(dcp->modes)) { dev_warn(dcp->dev, "failed to parse modes\n"); diff --git a/drivers/gpu/drm/apple/parser.c b/drivers/gpu/drm/apple/parser.c index 16027c1923369..73e7e1cf315c6 100644 --- a/drivers/gpu/drm/apple/parser.c +++ b/drivers/gpu/drm/apple/parser.c @@ -390,10 +390,32 @@ static int fill_color_mode(struct dcp_color_mode *color, return 0; } +/* External SDR scanout must not select a wide-gamut mode by score alone. */ +static unsigned int external_sdr_rank(const struct color_mode *color) +{ + if (color->id < 0 || color->id > U32_MAX || + color->eotf != DCP_EOTF_SDR_GAMMA || + color->pixel_encoding != DCP_COLOR_FORMAT_RGB || + color->dynamic_range != DCP_COLOR_YCBCR_RANGE_FULL || + (color->depth != 8 && color->depth != 10)) + return 0; + + switch (color->colorimetry) { + case DCP_COLORIMETRY_SRGB: + case DCP_COLORIMETRY_BT709: + return 2; + case DCP_COLORIMETRY_RGB: + return 1; + default: + return 0; + } +} + static int parse_color_modes(struct dcp_parse_ctx *handle, - struct dcp_display_mode *out) + struct dcp_display_mode *out, bool external_sdr) { struct iterator outer_it; + unsigned int best_sdr_rank = 0; int ret = 0; out->sdr_444.score = -1; out->sdr_rgb.score = -1; @@ -461,6 +483,18 @@ static int parse_color_modes(struct dcp_parse_ctx *handle, cmode.eotf, cmode.dynamic_range, cmode.pixel_encoding); + if (external_sdr) { + unsigned int rank = external_sdr_rank(&cmode); + + if (!rank || rank < best_sdr_rank) + continue; + if (rank > best_sdr_rank) + out->sdr_rgb.score = -1; + fill_color_mode(&out->sdr_rgb, &cmode); + best_sdr_rank = rank; + continue; + } + if (cmode.eotf == DCP_EOTF_SDR_GAMMA) { if (cmode.pixel_encoding == DCP_COLOR_FORMAT_RGB && cmode.depth <= 10) @@ -492,7 +526,7 @@ static u64 calculate_clock(struct dimension *horiz, struct dimension *vert) static int parse_mode(struct dcp_parse_ctx *handle, struct dcp_display_mode *out, s64 *score, int width_mm, - int height_mm, unsigned notch_height, bool internal) + int height_mm, unsigned notch_height, bool internal, bool external_sdr) { int ret = 0; struct iterator it; @@ -532,7 +566,7 @@ static int parse_mode(struct dcp_parse_ctx *handle, else if (!strcmp(key, "MaximumVariableRefreshRate")) ret = parse_int_bound(it.handle, &max_vrr, 0, U32_MAX); else if (!strcmp(key, "ColorModes")) - ret = parse_color_modes(it.handle, out); + ret = parse_color_modes(it.handle, out, external_sdr); else if (!strcmp(key, "ID")) ret = parse_int_bound(it.handle, &id, 0, U32_MAX); else if (!strcmp(key, "IsVirtual")) @@ -638,7 +672,7 @@ static int parse_mode(struct dcp_parse_ctx *handle, struct dcp_display_mode *enumerate_modes(struct dcp_parse_ctx *handle, unsigned int *count, int width_mm, int height_mm, unsigned notch_height, - bool internal) + bool internal, bool external_sdr) { struct iterator it; int ret; @@ -663,7 +697,7 @@ struct dcp_display_mode *enumerate_modes(struct dcp_parse_ctx *handle, mode = &modes[*count]; ret = parse_mode(it.handle, mode, &score, width_mm, height_mm, - notch_height, internal); + notch_height, internal, external_sdr); /* Errors for a single mode are recoverable -- just skip it. */ if (ret) { diff --git a/drivers/gpu/drm/apple/parser.h b/drivers/gpu/drm/apple/parser.h index bfb0a06c8760a..1f3b8782b7bfc 100644 --- a/drivers/gpu/drm/apple/parser.h +++ b/drivers/gpu/drm/apple/parser.h @@ -105,7 +105,7 @@ int parse(const void *blob, size_t size, struct dcp_parse_ctx *ctx); struct dcp_display_mode *enumerate_modes(struct dcp_parse_ctx *handle, unsigned int *count, int width_mm, int height_mm, unsigned notch_height, - bool internal); + bool internal, bool external_sdr); int parse_display_attributes(struct dcp_parse_ctx *handle, int *width_mm, int *height_mm); int parse_epic_service_init(struct dcp_parse_ctx *handle, const char **name, diff --git a/tools/diagnostics/test-apple-external-sdr.py b/tools/diagnostics/test-apple-external-sdr.py new file mode 100644 index 0000000000000..f72c26e38108c --- /dev/null +++ b/tools/diagnostics/test-apple-external-sdr.py @@ -0,0 +1,188 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only +"""Parse real-format TimingElements blobs with the production SDR selection code.""" +import hashlib +import os +from pathlib import Path +import re +import shlex +import struct +import subprocess +import tempfile + +root = Path(__file__).resolve().parents[2] +base = root / 'drivers/gpu/drm/apple' +source = (base / 'parser.c').read_text() +header = (base / 'parser.h').read_text() +body = source[source.index('#define DCP_PARSE_HEADER'):source.index('int parse_display_attributes(')] +header = header[header.index('struct apple_dcp;'):header.index('int parse(')] +# Pin the integration scope: the template passes its explicit external profile, +# independent of the existing internal-panel boolean. +template = (base / 'iomfb_template.c').read_text() +call = re.search(r'enumerate_modes\(&ctx,.*?\);', template, re.S).group() +assert re.search(r'dcp->hw\.iomfb_method_profile\s*==\s*DCP_IOMFB_METHODS_H17P_EXTERNAL_EXPERIMENTAL\)', call) +print('Production mode parser SHA256:', hashlib.sha256((header + body).encode()).hexdigest()) + + +def wire(value, out): + out += b'\0' * (-len(out) % 4) + if isinstance(value, bool): + out += struct.pack(' +#include +#include +#include +#include +#include +#include +#include +typedef uint8_t u8; +typedef uint32_t u32; +typedef uint64_t u64; +typedef int64_t s64; +#define __packed __attribute__((packed)) +#define IS_ENABLED(x) 0 +#define U32_MAX UINT32_MAX +#define U16_MAX UINT16_MAX +#define S64_MAX INT64_MAX +#define S64_MIN INT64_MIN +#define round_up(n, a) (((n) + (a) - 1) & ~((a) - 1)) +#define ERR_PTR(n) ((void *)(intptr_t)(n)) +#define PTR_ERR(p) ((int)(intptr_t)(p)) +#define IS_ERR(p) ((uintptr_t)(p) > (uintptr_t)-4096) +#define IS_ERR_OR_NULL(p) (!(p) || IS_ERR(p)) +#define GFP_KERNEL 0 +#define kmalloc(n, f) malloc(n) +#define kcalloc(n, s, f) calloc(n,s) +#define kfree(p) free(p) +#define mul_u32_u32(a, b) ((u64)(u32)(a) * (u32)(b)) +#define DIV_ROUND_CLOSEST_ULL(n, d) (((n) + (d) / 2) / (d)) +#define trace_iomfb_color_mode(...) ((void)0) +#define trace_iomfb_parse_mode_fail(...) ((void)0) +#define trace_iomfb_parse_mode_success(...) ((void)0) +#define trace_iomfb_timing_mode(...) ((void)0) +#define DRM_MODE_TYPE_DRIVER 1 +#define DRM_MODE_TYPE_PREFERRED 2 +struct drm_display_mode { + unsigned int type; + int clock, vdisplay, vsync_start, vsync_end, vtotal; + int hdisplay, hsync_start, hsync_end, htotal, width_mm, height_mm; +}; +static void drm_mode_set_name(struct drm_display_mode *mode) { (void)mode; } +''' +data = '\n'.join('static const unsigned char blob_%u[] = {%s};' % + (i, ','.join(map(str, case[1]))) for i, case in enumerate(cases)) +suffix = r''' +int main(void) +{ +''' +for i, (name, blob, external, count, identity, depth, clock) in enumerate(cases): + suffix += f''' + {{ + struct dcp_parse_ctx ctx; + unsigned int count = 99; + assert(parse(blob_{i}, sizeof(blob_{i}), &ctx) == 0); + struct dcp_display_mode *modes = enumerate_modes(&ctx, &count, 600, 340, 0, + {str(name.startswith('internal')).lower()}, {str(external).lower()}); + if (IS_ERR(modes) || count != {count}) {{ + fprintf(stderr, "case {i}: {name} count=%u\\n", count); + abort(); + }} +''' + if count: + suffix += f''' + assert(modes[0].color_mode_id == {identity}); + assert(modes[0].sdr_rgb.depth == {depth}); + assert(modes[0].mode.clock == {clock}); + assert(modes[0].mode.type & DRM_MODE_TYPE_PREFERRED); +''' + suffix += ' free(modes);\n }\n' +suffix += f' puts("PASS: {len(cases)} production binary-parser SDR, malformed-input, legacy and high-refresh cases");\n return 0;\n}}\n' +with tempfile.TemporaryDirectory(prefix='apple-external-sdr-') as directory: + directory = Path(directory) + unit = directory / 'test.c' + unit.write_text(prefix + header + body + data + suffix) + command = shlex.split(os.environ.get('CC', 'clang')) + command += ['-std=gnu11', '-Wall', '-Wextra', '-Werror', '-Wno-sign-compare', '-O1', '-g', + '-fsanitize=address,undefined', '-fno-omit-frame-pointer'] + subprocess.run(command + [str(unit), '-o', str(directory / 'test')], check=True) + subprocess.run([str(directory / 'test')], check=True) From 17be9546cc7503413e791b070aba8ae3ac055cba Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Mon, 12 Oct 2026 21:49:16 +1100 Subject: [PATCH 4/4] drm/apple: publish initial external discovery while presents remain paused The lifecycle gate cached the first connected HPD while waiting for DRM activation to unpause presentation. DRM could not activate a connector that had never been published, leaving the R35 external output absent even with a connected DPTX link and valid mode metadata. Publish cable-backed initial discovery before the first power attempt, while retaining the presentation pause. Sticky failures, absent cables and OFF/restart callbacks remain withheld. Receive callbacks still acquire no lifecycle mutex or issue firmware control requests. The regression test fails on the prior production function and passes with this change; it verifies that discovery does not power or unpause the pipe. ASan/UBSan lifecycle, metadata and delayed-service tests pass. Hardware activation and OFF-response qualification remain pending. Generated-by: LLM Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/gpu/drm/apple/dcp.c | 9 +++++++++ .../diagnostics/test-apple-external-lifecycle.py | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/apple/dcp.c b/drivers/gpu/drm/apple/dcp.c index 148f3ee692a0f..d01725a99103f 100644 --- a/drivers/gpu/drm/apple/dcp.c +++ b/drivers/gpu/drm/apple/dcp.c @@ -2671,6 +2671,15 @@ bool dcp_external_hotplug(struct apple_dcp *dcp, bool connected) if (dcp->iomfb.presentation_paused) { dcp->external_hotplug_pending = true; dcp->external_hotplug_connected = connected; + /* Publish discovery so DRM can activate the first CRTC; keep presents paused. */ + if (connected && READ_ONCE(dcp->external_cable) && + !READ_ONCE(dcp->external_power_attempted) && !READ_ONCE(dcp->external_error)) { + unsigned int action = dcp_mode_hotplug(&dcp->mode_state, true, + dcp->connector ? + &dcp->connector->connected : NULL); + + dcp_handle_hotplug_actions(dcp, action); + } return true; } if (!connected && !READ_ONCE(dcp->iomfb.presentation_paused)) { diff --git a/tools/diagnostics/test-apple-external-lifecycle.py b/tools/diagnostics/test-apple-external-lifecycle.py index b676b4b45076c..ca24415533bc3 100644 --- a/tools/diagnostics/test-apple-external-lifecycle.py +++ b/tools/diagnostics/test-apple-external-lifecycle.py @@ -475,16 +475,28 @@ def external_branch(name, result): assert(dcp_dptx_connect_oob(&dev,0)==-EAGAIN && p.typec_generation==1); assert(dcp_metadata_matches(&p.metadata_epoch,first)); - /* Link hookup alone never powers the pipe. A first activation preserves DA1 order. */ + /* Discovery publishes physical HPD before DRM activation, without unpausing the pipe. */ initial(&p); p.external_power_attempted = p.external_running = false; p.iomfb.presentation_paused = true; - p.dptxport[0].connected = false; + p.dptxport[0].connected = false; connector.connected=false; assert(dcp_dptx_connect_oob(&dev, 0) == 0 && connects == 1 && !power1); + assert(connector.connected && p.iomfb.presentation_paused); dcp_external_recover(&dev); assert(strstr(events, "DA1") && p.external_running && power1 == 1); owned(&p); + /* Discovery cannot publish a missing cable, a sticky error, or an OFF/restart callback. */ + for(unsigned int reason=0;reason<3;reason++) { + initial(&p); connector.connected=false; p.iomfb.presentation_paused=true; + p.external_power_attempted=false; + if(reason==0)p.external_cable=false; + if(reason==1)p.external_error=-EIO; + if(reason==2)p.external_power_attempted=true; + mutex_lock(&p.iomfb.lock); assert(dcp_external_hotplug(&p,true)); mutex_unlock(&p.iomfb.lock); + assert(!connector.connected && p.iomfb.presentation_paused && !power1); + } + /* Exact ON requires the 64-bit reply 1; all-zero must not activate modesets. */ initial(&p); p.external_running = false; p.external_stopped = true;