[PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank

Deng, Emily Emily.Deng at amd.com
Fri Sep 18 08:29:30 UTC 2020


Thanks, will double check.

Best wishes
Emily Deng



>-----Original Message-----
>From: Zhang, Hawking <Hawking.Zhang at amd.com>
>Sent: Friday, September 18, 2020 4:20 PM
>To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng at amd.com>
>Subject: RE: [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
>
>[AMD Public Use]
>
>+	spin_lock_irqsave(&adev->ddev->event_lock, flags);
>
>Are you sure you used the latest code base? I think recently we already switch
>to adev_to_drm(adev).
>
>Could you please double check?
>
>Regards,
>Hawking
>
>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
>Emily.Deng
>Sent: Friday, September 18, 2020 11:27
>To: amd-gfx at lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng at amd.com>
>Subject: [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
>
>Always start vblank timer, but only calls vblank function when vblank is enabled.
>
>This is used to fix the dead lock issue.
>When drm_crtc_vblank_off want to disable vblank, it first get event_lock, and
>then call hrtimer_cancel, but hrtimer_cancel want to wait timer handler
>function finished.
>Timer handler also want to aquire event_lock in drm_handle_vblank.
>
>Signed-off-by: Emily.Deng <Emily.Deng at amd.com>
>Change-Id: I7d3cfb1202cd030fdcdec3e7483fcc4c9fa8db70
>---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 155 +++++++++++------------
> 1 file changed, 77 insertions(+), 78 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>index cc93577dee03..8c02ab74c1de 100644
>--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>@@ -226,6 +226,74 @@ static const struct drm_crtc_helper_funcs
>dce_virtual_crtc_helper_funcs = {
> 	.get_scanout_position = amdgpu_crtc_get_scanout_position,  };
>
>+static int dce_virtual_pageflip(struct amdgpu_device *adev,
>+				unsigned crtc_id)
>+{
>+	unsigned long flags;
>+	struct amdgpu_crtc *amdgpu_crtc;
>+	struct amdgpu_flip_work *works;
>+
>+	amdgpu_crtc = adev->mode_info.crtcs[crtc_id];
>+
>+	if (crtc_id >= adev->mode_info.num_crtc) {
>+		DRM_ERROR("invalid pageflip crtc %d\n", crtc_id);
>+		return -EINVAL;
>+	}
>+
>+	/* IRQ could occur when in initial stage */
>+	if (amdgpu_crtc == NULL)
>+		return 0;
>+
>+	spin_lock_irqsave(&adev->ddev->event_lock, flags);
>+	works = amdgpu_crtc->pflip_works;
>+	if (amdgpu_crtc->pflip_status != AMDGPU_FLIP_SUBMITTED) {
>+		DRM_DEBUG_DRIVER("amdgpu_crtc->pflip_status = %d != "
>+			"AMDGPU_FLIP_SUBMITTED(%d)\n",
>+			amdgpu_crtc->pflip_status,
>+			AMDGPU_FLIP_SUBMITTED);
>+		spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>+		return 0;
>+	}
>+
>+	/* page flip completed. clean up */
>+	amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE;
>+	amdgpu_crtc->pflip_works = NULL;
>+
>+	/* wakeup usersapce */
>+	if (works->event)
>+		drm_crtc_send_vblank_event(&amdgpu_crtc->base, works-
>>event);
>+
>+	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>+
>+	drm_crtc_vblank_put(&amdgpu_crtc->base);
>+	amdgpu_bo_unref(&works->old_abo);
>+	kfree(works->shared);
>+	kfree(works);
>+
>+	return 0;
>+}
>+
>+static enum hrtimer_restart dce_virtual_vblank_timer_handle(struct
>+hrtimer *vblank_timer) {
>+	struct amdgpu_crtc *amdgpu_crtc = container_of(vblank_timer,
>+						       struct amdgpu_crtc,
>vblank_timer);
>+	struct drm_device *ddev = amdgpu_crtc->base.dev;
>+	struct amdgpu_device *adev = ddev->dev_private;
>+	struct amdgpu_irq_src *source = adev-
>>irq.client[AMDGPU_IRQ_CLIENTID_LEGACY].sources
>+		[VISLANDS30_IV_SRCID_SMU_DISP_TIMER2_TRIGGER];
>+	int irq_type = amdgpu_display_crtc_idx_to_irq_type(adev,
>+						amdgpu_crtc->crtc_id);
>+
>+	if (amdgpu_irq_enabled(adev, source, irq_type)) {
>+		drm_handle_vblank(ddev, amdgpu_crtc->crtc_id);
>+		dce_virtual_pageflip(adev, amdgpu_crtc->crtc_id);
>+	}
>+	hrtimer_start(vblank_timer, ktime_set(0,
>DCE_VIRTUAL_VBLANK_PERIOD),
>+		      HRTIMER_MODE_REL);
>+
>+	return HRTIMER_NORESTART;
>+}
>+
> static int dce_virtual_crtc_init(struct amdgpu_device *adev, int index)  {
> 	struct amdgpu_crtc *amdgpu_crtc;
>@@ -247,6 +315,14 @@ static int dce_virtual_crtc_init(struct amdgpu_device
>*adev, int index)
> 	amdgpu_crtc->vsync_timer_enabled = AMDGPU_IRQ_STATE_DISABLE;
> 	drm_crtc_helper_add(&amdgpu_crtc->base,
>&dce_virtual_crtc_helper_funcs);
>
>+	hrtimer_init(&amdgpu_crtc->vblank_timer,
>+		     CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>+	hrtimer_set_expires(&amdgpu_crtc->vblank_timer,
>+			    ktime_set(0, DCE_VIRTUAL_VBLANK_PERIOD));
>+	amdgpu_crtc->vblank_timer.function =
>+		dce_virtual_vblank_timer_handle;
>+	hrtimer_start(&amdgpu_crtc->vblank_timer,
>+			      ktime_set(0, DCE_VIRTUAL_VBLANK_PERIOD),
>HRTIMER_MODE_REL);
> 	return 0;
> }
>
>@@ -476,7 +552,7 @@ static int dce_virtual_hw_fini(void *handle)
>
> 	for (i = 0; i<adev->mode_info.num_crtc; i++)
> 		if (adev->mode_info.crtcs[i])
>-			dce_virtual_set_crtc_vblank_interrupt_state(adev, i,
>AMDGPU_IRQ_STATE_DISABLE);
>+			hrtimer_cancel(&adev->mode_info.crtcs[i]-
>>vblank_timer);
>
> 	return 0;
> }
>@@ -645,68 +721,6 @@ static void dce_virtual_set_display_funcs(struct
>amdgpu_device *adev)
> 	adev->mode_info.funcs = &dce_virtual_display_funcs;  }
>
>-static int dce_virtual_pageflip(struct amdgpu_device *adev,
>-				unsigned crtc_id)
>-{
>-	unsigned long flags;
>-	struct amdgpu_crtc *amdgpu_crtc;
>-	struct amdgpu_flip_work *works;
>-
>-	amdgpu_crtc = adev->mode_info.crtcs[crtc_id];
>-
>-	if (crtc_id >= adev->mode_info.num_crtc) {
>-		DRM_ERROR("invalid pageflip crtc %d\n", crtc_id);
>-		return -EINVAL;
>-	}
>-
>-	/* IRQ could occur when in initial stage */
>-	if (amdgpu_crtc == NULL)
>-		return 0;
>-
>-	spin_lock_irqsave(&adev_to_drm(adev)->event_lock, flags);
>-	works = amdgpu_crtc->pflip_works;
>-	if (amdgpu_crtc->pflip_status != AMDGPU_FLIP_SUBMITTED) {
>-		DRM_DEBUG_DRIVER("amdgpu_crtc->pflip_status = %d != "
>-			"AMDGPU_FLIP_SUBMITTED(%d)\n",
>-			amdgpu_crtc->pflip_status,
>-			AMDGPU_FLIP_SUBMITTED);
>-		spin_unlock_irqrestore(&adev_to_drm(adev)->event_lock,
>flags);
>-		return 0;
>-	}
>-
>-	/* page flip completed. clean up */
>-	amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE;
>-	amdgpu_crtc->pflip_works = NULL;
>-
>-	/* wakeup usersapce */
>-	if (works->event)
>-		drm_crtc_send_vblank_event(&amdgpu_crtc->base, works-
>>event);
>-
>-	spin_unlock_irqrestore(&adev_to_drm(adev)->event_lock, flags);
>-
>-	drm_crtc_vblank_put(&amdgpu_crtc->base);
>-	amdgpu_bo_unref(&works->old_abo);
>-	kfree(works->shared);
>-	kfree(works);
>-
>-	return 0;
>-}
>-
>-static enum hrtimer_restart dce_virtual_vblank_timer_handle(struct hrtimer
>*vblank_timer) -{
>-	struct amdgpu_crtc *amdgpu_crtc = container_of(vblank_timer,
>-						       struct amdgpu_crtc,
>vblank_timer);
>-	struct drm_device *ddev = amdgpu_crtc->base.dev;
>-	struct amdgpu_device *adev = drm_to_adev(ddev);
>-
>-	drm_handle_vblank(ddev, amdgpu_crtc->crtc_id);
>-	dce_virtual_pageflip(adev, amdgpu_crtc->crtc_id);
>-	hrtimer_start(vblank_timer, DCE_VIRTUAL_VBLANK_PERIOD,
>-		      HRTIMER_MODE_REL);
>-
>-	return HRTIMER_NORESTART;
>-}
>-
> static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device
>*adev,
> 							int crtc,
> 							enum
>amdgpu_interrupt_state state) @@ -716,21 +730,6 @@ static void
>dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *ad
> 		return;
> 	}
>
>-	if (state && !adev->mode_info.crtcs[crtc]->vsync_timer_enabled) {
>-		DRM_DEBUG("Enable software vsync timer\n");
>-		hrtimer_init(&adev->mode_info.crtcs[crtc]->vblank_timer,
>-			     CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>-		hrtimer_set_expires(&adev->mode_info.crtcs[crtc]-
>>vblank_timer,
>-				    DCE_VIRTUAL_VBLANK_PERIOD);
>-		adev->mode_info.crtcs[crtc]->vblank_timer.function =
>-			dce_virtual_vblank_timer_handle;
>-		hrtimer_start(&adev->mode_info.crtcs[crtc]->vblank_timer,
>-			      DCE_VIRTUAL_VBLANK_PERIOD,
>HRTIMER_MODE_REL);
>-	} else if (!state && adev->mode_info.crtcs[crtc]->vsync_timer_enabled)
>{
>-		DRM_DEBUG("Disable software vsync timer\n");
>-		hrtimer_cancel(&adev->mode_info.crtcs[crtc]->vblank_timer);
>-	}
>-
> 	adev->mode_info.crtcs[crtc]->vsync_timer_enabled = state;
> 	DRM_DEBUG("[FM]set crtc %d vblank interrupt state %d\n", crtc,
>state);  }
>--
>2.25.1
>
>_______________________________________________
>amd-gfx mailing list
>amd-gfx at lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fre
>edesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfx&data=02%7C01%7Chawking.zhang%40amd.com%7C4aa2942fb0bd4a
>25c66a08d85b82c813%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0
>%7C637359964549266369&sdata=1ohOBjPciizMDNdCYnMUj9e160K%2F
>QyKzpgmmEYhCIOM%3D&reserved=0


More information about the amd-gfx mailing list