drm/client: Make the FBIO_WAITFORVSYNC vblank wait interruptible

index 0080a8e9556a..b475b8dbdb17 100644
@@ -1301,10 +1301,11 @@ EXPORT_SYMBOL(drm_client_modeset_dpms);
  *
  * Block the caller until the given CRTC has seen a VBLANK. Do nothing
  * if the CRTC is disabled. If there's another DRM master present, fail
- * with -EBUSY.
+ * with -EBUSY. A signal interrupts the wait.
  *
  * Returns:
- * 0 on success, or negative error code otherwise.
+ * 0 on success, -ERESTARTSYS if a signal interrupted the wait, or
+ * negative error code otherwise.
  */
 int drm_client_modeset_wait_for_vblank(struct drm_client_dev *client, unsigned int crtc_index)
 {

@@ -1328,13 +1329,14 @@ int drm_client_modeset_wait_for_vblank(struct drm_client_dev *client, unsigned i
 	 */
 	ret = drm_crtc_vblank_get(crtc);
 	if (!ret) {
-		drm_crtc_wait_one_vblank(crtc);
+		ret = drm_crtc_wait_one_vblank_interruptible(crtc);
 		drm_crtc_vblank_put(crtc);
 	}
 
 	drm_master_internal_release(dev);
 
-	return 0;
+	/* Only report interruption by a signal, not timeouts. */
+	return ret == -ERESTARTSYS ? ret : 0;
 }
 EXPORT_SYMBOL(drm_client_modeset_wait_for_vblank);
 

index f893b1e3a596..2be21c20eb78 100644
@@ -115,6 +115,7 @@ void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe);
 int drm_vblank_get(struct drm_device *dev, unsigned int pipe);
 void drm_vblank_put(struct drm_device *dev, unsigned int pipe);
 u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe);
+int drm_crtc_wait_one_vblank_interruptible(struct drm_crtc *crtc);
 
 /* drm_vblank_work.c */
 static inline void drm_vblank_flush_worker(struct drm_vblank_crtc *vblank)

83 lines not on this page · raw message

index c3c9afc08ed2..5e247663978a 100644
@@ -1315,21 +1315,29 @@ static int __drm_crtc_wait_one_vblank(struct drm_crtc *crtc, bool interruptible)
 
 	if (interruptible)
 		timeout = wait_event_interruptible_timeout(vblank->queue,
-							   last != drm_vblank_count(dev, pipe),
+							   last != drm_vblank_count(dev, pipe) ||
+							   !READ_ONCE(vblank->enabled),
 							   timeout);
 	else
 		timeout = wait_event_timeout(vblank->queue,
-					     last != drm_vblank_count(dev, pipe),
+					     last != drm_vblank_count(dev, pipe) ||
+					     !READ_ONCE(vblank->enabled),
 					     timeout);
 
 	drm_WARN(dev, timeout == 0, "vblank wait timed out on crtc %i\n", pipe);
 
-	drm_vblank_put(dev, pipe);
-
 	if (timeout < 0)
-		return timeout;
+		ret = timeout;
+	else if (!timeout)
+		ret = -ETIMEDOUT;
+	else if (last == drm_vblank_count(dev, pipe))
+		ret = -EINVAL; /* drm_crtc_vblank_off() ended the wait */
+	else
+		ret = 0;
 
-	return timeout ? 0 : -ETIMEDOUT;
+	drm_vblank_put(dev, pipe);
+
+	return ret;
 }
 
 /**

@@ -1338,7 +1346,8 @@ static int __drm_crtc_wait_one_vblank(struct drm_crtc *crtc, bool interruptible)
  *
  * This waits for one vblank to pass on @crtc, using the irq driver interfaces.
  * It is a failure to call this when the vblank irq for @crtc is disabled, e.g.
- * due to lack of driver support or because the crtc is off.
+ * due to lack of driver support or because the crtc is off. The wait also fails
+ * if drm_crtc_vblank_off() disables the vblank irq before a vblank passes.
  *
  * Returns: 0 on success, negative error on failures.
  */

index 33c73ca9cff1..d3882ffcb5d4 100644
@@ -3825,6 +3825,11 @@ EXPORT_IF_KUNIT(dm_arm_vblank_event);
  * Grab a reference on the vblank counter if a page flip or cursor update is to
  * be programmed. Do this before programming so the HW is not in any
  * idle-optimized state (such as PSR).
+ *
+ * If vblank is off, no vblank interrupt can deliver the event, and the
+ * handlers that send it would drop a reference that was never taken. Send
+ * the event right away instead, like drm_crtc_vblank_off() does for queued
+ * vblank events.
  */
 STATIC_IFN_KUNIT void dm_arm_vblank_event_pre_programming(struct amdgpu_crtc *acrtc,
 							  struct dm_crtc_state *acrtc_state,

@@ -3836,8 +3841,14 @@ STATIC_IFN_KUNIT void dm_arm_vblank_event_pre_programming(struct amdgpu_crtc *ac
 	if (!acrtc->base.state->event || acrtc_state->active_planes == 0)
 		return;
 
-	if (pflip_update || cursor_update)
-		drm_crtc_vblank_get(&acrtc->base);
+	if (!pflip_update && !cursor_update)
+		return;
+
+	if (drm_crtc_vblank_get(&acrtc->base)) {
+		drm_crtc_send_vblank_event(&acrtc->base,
+					   acrtc->base.state->event);
+		acrtc->base.state->event = NULL;
+	}
 }
 EXPORT_IF_KUNIT(dm_arm_vblank_event_pre_programming);
 

index b9272b28f53c..ebec1260c510 100644
@@ -1812,6 +1812,34 @@ static void dm_test_arm_vblank_pre_programming_update(struct kunit *test)
 	drm_crtc_vblank_put(&ctx->acrtc->base);
 }
 
+/**
+ * dm_test_arm_vblank_pre_programming_vblank_off - Test the event is sent with vblank off
+ * @test: The KUnit test context
+ *
+ * No vblank interrupt can deliver the event, and the handlers that send it
+ * must not drop a vblank reference that was never taken.
+ */
+static void dm_test_arm_vblank_pre_programming_vblank_off(struct kunit *test)
+{
+	struct dm_test_vblank_ctx *ctx = dm_test_vblank_ctx_alloc(test);
+	struct drm_vblank_crtc *vblank;
+
+	KUNIT_ASSERT_EQ(test, drm_vblank_init(&ctx->adev->ddev, 1), 0);
+	vblank = drm_crtc_vblank_crtc(&ctx->acrtc->base);
+	drm_crtc_vblank_off(&ctx->acrtc->base);
+
+	/* Sending an event that has no file frees it. */
+	ctx->acrtc_state->base.event = kzalloc_obj(*ctx->event, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->acrtc_state->base.event);
+
+	dm_test_arm_vblank_pre_programming(ctx, true, false);
+
+	KUNIT_EXPECT_NULL(test, ctx->acrtc_state->base.event);
+	/* Only drm_crtc_vblank_off() holds a reference. */
+	KUNIT_EXPECT_EQ(test, atomic_read(&vblank->refcount), 1);
+	kfree(ctx->acrtc_state->base.event);
+}
+
 /**
  * dm_test_arm_vblank_event_no_event - Test a commit without an event is a no-op
  * @test: The KUnit test context

@@ -5298,6 +5326,7 @@ static struct kunit_case amdgpu_dm_tests[] = {
 	KUNIT_CASE(dm_test_arm_vblank_pre_programming_no_event),
 	KUNIT_CASE(dm_test_arm_vblank_pre_programming_no_planes),
 	KUNIT_CASE(dm_test_arm_vblank_pre_programming_update),
+	KUNIT_CASE(dm_test_arm_vblank_pre_programming_vblank_off),
 	/* dm_update_pflip_irq_state */
 	KUNIT_CASE(dm_test_update_pflip_irq_state_dcn),
 	KUNIT_CASE(dm_test_update_pflip_irq_state_dce),

index d3882ffcb5d4..007968e892c8 100644
@@ -1496,6 +1496,56 @@ STATIC_IFN_KUNIT void dm_gpureset_toggle_interrupts(struct amdgpu_device *adev,
 }
 EXPORT_IF_KUNIT(dm_gpureset_toggle_interrupts);
 
+/**
+ * dm_gpureset_toggle_vblank() - Turn DRM vblank off or on around a GPU reset
+ * @adev: amdgpu device
+ * @state: DC state whose streams the reset stops and restores
+ * @enable: false before the streams are stopped, true once they are restored
+ *
+ * The GPU reset path stops and restores the streams through DC directly and
+ * bypasses manage_dm_interrupts(). Turn vblank off in DRM like a CRTC
+ * disable does, so that drm_crtc_vblank_get() fails while no vblank
+ * interrupt can arrive, instead of letting vblank waiters such as the fbdev
+ * damage worker sleep until they time out.
+ */
+STATIC_IFN_KUNIT void dm_gpureset_toggle_vblank(struct amdgpu_device *adev,
+						struct dc_state *state, bool enable)
+{
+	struct drm_vblank_crtc_config config;
+	struct amdgpu_crtc *acrtc;
+	int i;
+
+	for (i = 0; i < state->stream_count; i++) {
+		int otg_inst = state->stream_status[i].primary_otg_inst;
+
+		acrtc = amdgpu_dm_get_crtc_by_otg_inst(adev, otg_inst);
+		if (!acrtc)
+			continue;
+
+		if (!enable) {
+			drm_crtc_vblank_off(&acrtc->base);
+			continue;
+		}
+
+		/*
+		 * A modeset waiting for dc_lock may be turning the CRTC off,
+		 * and amdgpu_dm_crtc_set_vblank() refuses to enable vblank on
+		 * it then. Leave its vblank off.
+		 */
+		if (!acrtc->base.enabled)
+			continue;
+
+		/* drm_crtc_vblank_off() cleared the timestamping constants. */
+		drm_calc_timestamping_constants(&acrtc->base,
+						&acrtc->base.state->adjusted_mode);
+
+		/* Keep the off delay that manage_dm_interrupts() configured. */
+		config = drm_crtc_vblank_crtc(&acrtc->base)->config;
+		drm_crtc_vblank_on_config(&acrtc->base, &config);
+	}
+}
+EXPORT_IF_KUNIT(dm_gpureset_toggle_vblank);
+
 DEFINE_FREE(state_release, struct dc_state *, dc_state_release(_T))
 
 STATIC_IFN_KUNIT enum dc_status amdgpu_dm_commit_zero_streams(struct dc *dc)

@@ -1630,6 +1680,7 @@ static int dm_suspend(struct amdgpu_ip_block *ip_block)
 			return -ENOMEM;
 		}
 
+		dm_gpureset_toggle_vblank(adev, dm->cached_dc_state, false);
 		dm_gpureset_toggle_interrupts(adev, dm->cached_dc_state, false);
 
 		res = amdgpu_dm_commit_zero_streams(dm->dc);

@@ -1969,6 +2020,7 @@ static int dm_resume(struct amdgpu_ip_block *ip_block)
 		dm_gpureset_commit_state(dm->cached_dc_state, dm);
 
 		dm_gpureset_toggle_interrupts(adev, dm->cached_dc_state, true);
+		dm_gpureset_toggle_vblank(adev, dm->cached_dc_state, true);
 
 		dc_state_release(dm->cached_dc_state);
 		dm->cached_dc_state = NULL;

index f8503ebf28f4..303e9a57e7b8 100644
@@ -1351,6 +1351,7 @@ void mmhub_read_system_context(struct amdgpu_device *adev,
 			       struct dc_phy_addr_space_config *pa_config);
 int amdgpu_dm_init_power_module(struct amdgpu_display_manager *dm);
 void dm_gpureset_toggle_interrupts(struct amdgpu_device *adev, struct dc_state *state, bool enable);
+void dm_gpureset_toggle_vblank(struct amdgpu_device *adev, struct dc_state *state, bool enable);
 enum dc_status amdgpu_dm_commit_zero_streams(struct dc *dc);
 int dm_cache_state(struct amdgpu_device *adev);
 void dm_destroy_cached_state(struct amdgpu_device *adev);

124 lines not on this page · raw message

index 14a72a406d37..06e539c4c3ea 100644
@@ -1692,6 +1692,7 @@ u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = crtc->index;
 	struct amdgpu_device *adev = drm_to_adev(dev);
+	bool locked = false;
 	int vpos, hpos, stat;
 	u32 count;
 

@@ -1699,9 +1700,26 @@ u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc)
 		DRM_ERROR("Invalid crtc %u\n", pipe);
 		return -EINVAL;
 	}
-	if (!down_read_trylock(&adev->reset_domain->sem))
-		return drm_crtc_vblank_count(crtc);
 
+	/*
+	 * A GPU reset is in progress and the display hardware cannot be read
+	 * safely. Return the hardware counter value DRM already cached rather
+	 * than the software vblank count: the core feeds this callback's
+	 * result back into vblank->last, and a software count lives in a
+	 * different numeric domain, which later trips
+	 * drm_WARN_ON_ONCE(cur_vblank != vblank->last) in
+	 * drm_update_vblank_count().
+	 *
+	 * Only task context has to back off. Vblank interrupts only arrive
+	 * while a CRTC is scanning out, so the interrupt path can read the
+	 * hardware, and it has to keep counting: vblank waits that already
+	 * sleep when a reset takes the semaphore never complete otherwise.
+	 */
+	if (in_task()) {
+		if (!down_read_trylock(&adev->reset_domain->sem))
+			return drm_crtc_vblank_crtc(crtc)->last;
+		locked = true;
+	}
 
 	/* The hw increments its frame counter at start of vsync, not at start
 	 * of vblank, as is required by DRM core vblank counter handling.

@@ -1747,7 +1765,8 @@ u32 amdgpu_get_vblank_counter_kms(struct drm_crtc *crtc)
 		DRM_DEBUG_VBL("NULL mode info! Returned count may be wrong.\n");
 	}
 
-	up_read(&adev->reset_domain->sem);
+	if (locked)
+		up_read(&adev->reset_domain->sem);
 	return count;
 }