All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
@ 2020-09-18  3:27 Emily.Deng
  2020-09-18  3:27 ` [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank Emily.Deng
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Emily.Deng @ 2020-09-18  3:27 UTC (permalink / raw)
  To: amd-gfx; +Cc: Emily.Deng

For debug convenient, add sriov_mcbp parameter.

Signed-off-by: Emily.Deng <Emily.Deng@amd.com>
Change-Id: I84019eb4344e00d85b2ecc853145aabb312412fe
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..a255fbf4d370 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -183,6 +183,7 @@ extern uint amdgpu_ras_mask;
 extern int amdgpu_bad_page_threshold;
 extern int amdgpu_async_gfx_ring;
 extern int amdgpu_mcbp;
+extern int amdgpu_sriov_mcbp;
 extern int amdgpu_discovery;
 extern int amdgpu_mes;
 extern int amdgpu_noretry;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3f07d1475bd2..b0b2f0f7be94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -145,6 +145,7 @@ uint amdgpu_dc_feature_mask = 0;
 uint amdgpu_dc_debug_mask = 0;
 int amdgpu_async_gfx_ring = 1;
 int amdgpu_mcbp = 0;
+int amdgpu_sriov_mcbp = 1;
 int amdgpu_discovery = -1;
 int amdgpu_mes = 0;
 int amdgpu_noretry;
@@ -578,6 +579,14 @@ MODULE_PARM_DESC(mcbp,
 	"Enable Mid-command buffer preemption (0 = disabled (default), 1 = enabled)");
 module_param_named(mcbp, amdgpu_mcbp, int, 0444);
 
+/**
+ * DOC: sriov_mcbp (int)
+ * It is used to enable mid command buffer preemption. (0 = disabled, 1 = enabled(default))
+ */
+MODULE_PARM_DESC(sriov_mcbp,
+	"Enable sriov Mid-command buffer preemption (0 = disabled (default), 1 = enabled)");
+module_param_named(sriov_mcbp, amdgpu_sriov_mcbp, int, 0444);
+
 /**
  * DOC: discovery (int)
  * Allow driver to discover hardware IP information from IP Discovery table at the top of VRAM.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 2f53fa0ae9a6..ca0e17688bdf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -236,7 +236,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 
 	for (i = 0; i < num_ibs; ++i) {
 		ib = &ibs[i];
-
+		if (!amdgpu_sriov_mcbp)
+			ib->flags &= ~AMDGPU_IB_FLAG_PREEMPT;
 		/* drop preamble IBs if we don't have a context switch */
 		if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
 		    skip_preamble &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d7f37cb92a97..156e76a5a6e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -742,7 +742,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 		dev_info.ids_flags = 0;
 		if (adev->flags & AMD_IS_APU)
 			dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
-		if (amdgpu_mcbp || amdgpu_sriov_vf(adev))
+		if (amdgpu_mcbp || (amdgpu_sriov_vf(adev) && amdgpu_sriov_mcbp))
 			dev_info.ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;
 		if (amdgpu_is_tmz(adev))
 			dev_info.ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
  2020-09-18  3:27 [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Emily.Deng
@ 2020-09-18  3:27 ` Emily.Deng
  2020-09-18  8:20   ` Zhang, Hawking
  2020-09-21  4:04 ` [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Deng, Emily
  2020-09-21  5:37 ` Liu, Monk
  2 siblings, 1 reply; 7+ messages in thread
From: Emily.Deng @ 2020-09-18  3:27 UTC (permalink / raw)
  To: amd-gfx; +Cc: Emily.Deng

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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
  2020-09-18  3:27 ` [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank Emily.Deng
@ 2020-09-18  8:20   ` Zhang, Hawking
  2020-09-18  8:29     ` Deng, Emily
  0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Hawking @ 2020-09-18  8:20 UTC (permalink / raw)
  To: Deng, Emily, amd-gfx; +Cc: Deng, Emily

[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@lists.freedesktop.org> On Behalf Of Emily.Deng
Sent: Friday, September 18, 2020 11:27
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily <Emily.Deng@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@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@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Chawking.zhang%40amd.com%7C4aa2942fb0bd4a25c66a08d85b82c813%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359964549266369&amp;sdata=1ohOBjPciizMDNdCYnMUj9e160K%2FQyKzpgmmEYhCIOM%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
  2020-09-18  8:20   ` Zhang, Hawking
@ 2020-09-18  8:29     ` Deng, Emily
  0 siblings, 0 replies; 7+ messages in thread
From: Deng, Emily @ 2020-09-18  8:29 UTC (permalink / raw)
  To: Zhang, Hawking, amd-gfx

Thanks, will double check.

Best wishes
Emily Deng



>-----Original Message-----
>From: Zhang, Hawking <Hawking.Zhang@amd.com>
>Sent: Friday, September 18, 2020 4:20 PM
>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@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@lists.freedesktop.org> On Behalf Of
>Emily.Deng
>Sent: Friday, September 18, 2020 11:27
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@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@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@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fre
>edesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfx&amp;data=02%7C01%7Chawking.zhang%40amd.com%7C4aa2942fb0bd4a
>25c66a08d85b82c813%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0
>%7C637359964549266369&amp;sdata=1ohOBjPciizMDNdCYnMUj9e160K%2F
>QyKzpgmmEYhCIOM%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
  2020-09-18  3:27 [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Emily.Deng
  2020-09-18  3:27 ` [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank Emily.Deng
@ 2020-09-21  4:04 ` Deng, Emily
  2020-09-21  5:37 ` Liu, Monk
  2 siblings, 0 replies; 7+ messages in thread
From: Deng, Emily @ 2020-09-21  4:04 UTC (permalink / raw)
  To: Deng, Emily, amd-gfx

[AMD Official Use Only - Internal Distribution Only]

Ping .....

>-----Original Message-----
>From: Emily.Deng <Emily.Deng@amd.com>
>Sent: Friday, September 18, 2020 11:27 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@amd.com>
>Subject: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
>
>For debug convenient, add sriov_mcbp parameter.
>
>Signed-off-by: Emily.Deng <Emily.Deng@amd.com>
>Change-Id: I84019eb4344e00d85b2ecc853145aabb312412fe
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++
>drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 3 ++-
>drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> 4 files changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>index 13f92dea182a..a255fbf4d370 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>@@ -183,6 +183,7 @@ extern uint amdgpu_ras_mask;  extern int
>amdgpu_bad_page_threshold;  extern int amdgpu_async_gfx_ring;  extern int
>amdgpu_mcbp;
>+extern int amdgpu_sriov_mcbp;
> extern int amdgpu_discovery;
> extern int amdgpu_mes;
> extern int amdgpu_noretry;
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>index 3f07d1475bd2..b0b2f0f7be94 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>@@ -145,6 +145,7 @@ uint amdgpu_dc_feature_mask = 0;  uint
>amdgpu_dc_debug_mask = 0;  int amdgpu_async_gfx_ring = 1;  int
>amdgpu_mcbp = 0;
>+int amdgpu_sriov_mcbp = 1;
> int amdgpu_discovery = -1;
> int amdgpu_mes = 0;
> int amdgpu_noretry;
>@@ -578,6 +579,14 @@ MODULE_PARM_DESC(mcbp,
> "Enable Mid-command buffer preemption (0 = disabled (default), 1 =
>enabled)");  module_param_named(mcbp, amdgpu_mcbp, int, 0444);
>
>+/**
>+ * DOC: sriov_mcbp (int)
>+ * It is used to enable mid command buffer preemption. (0 = disabled, 1
>+= enabled(default))  */ MODULE_PARM_DESC(sriov_mcbp,
>+"Enable sriov Mid-command buffer preemption (0 = disabled (default),
>1
>+= enabled)"); module_param_named(sriov_mcbp, amdgpu_sriov_mcbp, int,
>+0444);
>+
> /**
>  * DOC: discovery (int)
>  * Allow driver to discover hardware IP information from IP Discovery table at
>the top of VRAM.
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>index 2f53fa0ae9a6..ca0e17688bdf 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>@@ -236,7 +236,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>unsigned num_ibs,
>
> for (i = 0; i < num_ibs; ++i) {
> ib = &ibs[i];
>-
>+if (!amdgpu_sriov_mcbp)
>+ib->flags &= ~AMDGPU_IB_FLAG_PREEMPT;
> /* drop preamble IBs if we don't have a context switch */
> if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
>     skip_preamble &&
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>index d7f37cb92a97..156e76a5a6e0 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>@@ -742,7 +742,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev,
>void *data, struct drm_file
> dev_info.ids_flags = 0;
> if (adev->flags & AMD_IS_APU)
> dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
>-if (amdgpu_mcbp || amdgpu_sriov_vf(adev))
>+if (amdgpu_mcbp || (amdgpu_sriov_vf(adev) &&
>amdgpu_sriov_mcbp))
> dev_info.ids_flags |=
>AMDGPU_IDS_FLAGS_PREEMPTION;
> if (amdgpu_is_tmz(adev))
> dev_info.ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
>--
>2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
  2020-09-18  3:27 [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Emily.Deng
  2020-09-18  3:27 ` [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank Emily.Deng
  2020-09-21  4:04 ` [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Deng, Emily
@ 2020-09-21  5:37 ` Liu, Monk
  2020-09-21  6:26   ` Deng, Emily
  2 siblings, 1 reply; 7+ messages in thread
From: Liu, Monk @ 2020-09-21  5:37 UTC (permalink / raw)
  To: Deng, Emily, amd-gfx; +Cc: Deng, Emily

[AMD Official Use Only - Internal Distribution Only]

Hi Emily

There is already a amdgpu_mcbp parameter there, can you try to leverage that one ?

e.g.:
we refactor our driver's code and reduce the checking logic  from  "if (amdgpu_mcbp || amdgpu_sriov_vf(adev))" to something like "if( amdgpu_mcbp) "

therefore:
1) You need to force set "amdgpu_mcbp" to true in the driver's init stage once the "SRIOV" is detected *and* "amdgpu_mcbp" is not set to "0";
2) for Bare-metal, we just leave "amdgpu_mcbp" as the value it was....
3) we interpret  "amdgpu_mcbp"  as:
0: force disable, it will be "disable" for both BM and SRIOV
1:  force enable, auto (default), it will be "enable" for both BM and SRIOV

This way if you can disable MCBP in both SRIOV and BM by that existed parameter instead of introducing a duplicated one ...

_____________________________________
Monk Liu|GPU Virtualization Team |AMD


-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Emily.Deng
Sent: Friday, September 18, 2020 11:27 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily <Emily.Deng@amd.com>
Subject: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug

For debug convenient, add sriov_mcbp parameter.

Signed-off-by: Emily.Deng <Emily.Deng@amd.com>
Change-Id: I84019eb4344e00d85b2ecc853145aabb312412fe
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 3 ++-  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..a255fbf4d370 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -183,6 +183,7 @@ extern uint amdgpu_ras_mask;  extern int amdgpu_bad_page_threshold;  extern int amdgpu_async_gfx_ring;  extern int amdgpu_mcbp;
+extern int amdgpu_sriov_mcbp;
 extern int amdgpu_discovery;
 extern int amdgpu_mes;
 extern int amdgpu_noretry;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3f07d1475bd2..b0b2f0f7be94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -145,6 +145,7 @@ uint amdgpu_dc_feature_mask = 0;  uint amdgpu_dc_debug_mask = 0;  int amdgpu_async_gfx_ring = 1;  int amdgpu_mcbp = 0;
+int amdgpu_sriov_mcbp = 1;
 int amdgpu_discovery = -1;
 int amdgpu_mes = 0;
 int amdgpu_noretry;
@@ -578,6 +579,14 @@ MODULE_PARM_DESC(mcbp,
 "Enable Mid-command buffer preemption (0 = disabled (default), 1 = enabled)");  module_param_named(mcbp, amdgpu_mcbp, int, 0444);

+/**
+ * DOC: sriov_mcbp (int)
+ * It is used to enable mid command buffer preemption. (0 = disabled, 1
+= enabled(default))  */ MODULE_PARM_DESC(sriov_mcbp,
+"Enable sriov Mid-command buffer preemption (0 = disabled (default), 1
+= enabled)"); module_param_named(sriov_mcbp, amdgpu_sriov_mcbp, int,
+0444);
+
 /**
  * DOC: discovery (int)
  * Allow driver to discover hardware IP information from IP Discovery table at the top of VRAM.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 2f53fa0ae9a6..ca0e17688bdf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -236,7 +236,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,

 for (i = 0; i < num_ibs; ++i) {
 ib = &ibs[i];
-
+if (!amdgpu_sriov_mcbp)
+ib->flags &= ~AMDGPU_IB_FLAG_PREEMPT;
 /* drop preamble IBs if we don't have a context switch */
 if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
     skip_preamble &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d7f37cb92a97..156e76a5a6e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -742,7 +742,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 dev_info.ids_flags = 0;
 if (adev->flags & AMD_IS_APU)
 dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
-if (amdgpu_mcbp || amdgpu_sriov_vf(adev))
+if (amdgpu_mcbp || (amdgpu_sriov_vf(adev) && amdgpu_sriov_mcbp))
 dev_info.ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;
 if (amdgpu_is_tmz(adev))
 dev_info.ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
--
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cmonk.liu%40amd.com%7C497edf57e53b41a055fc08d85b82c709%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637359964547689160&amp;sdata=zXlPTG939tdC0sUbCntiJuGsZHpM15DqWmgy8SmZ2Z8%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
  2020-09-21  5:37 ` Liu, Monk
@ 2020-09-21  6:26   ` Deng, Emily
  0 siblings, 0 replies; 7+ messages in thread
From: Deng, Emily @ 2020-09-21  6:26 UTC (permalink / raw)
  To: Liu, Monk, amd-gfx

[AMD Official Use Only - Internal Distribution Only]

Hi Monk,
    Good suggestion, will send out patch again.

Best wishes
Emily Deng



>-----Original Message-----
>From: Liu, Monk <Monk.Liu@amd.com>
>Sent: Monday, September 21, 2020 1:37 PM
>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@amd.com>
>Subject: RE: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp
>debug
>
>[AMD Official Use Only - Internal Distribution Only]
>
>Hi Emily
>
>There is already a amdgpu_mcbp parameter there, can you try to leverage that
>one ?
>
>e.g.:
>we refactor our driver's code and reduce the checking logic  from  "if
>(amdgpu_mcbp || amdgpu_sriov_vf(adev))" to something like
>"if( amdgpu_mcbp) "
>
>therefore:
>1) You need to force set "amdgpu_mcbp" to true in the driver's init stage once
>the "SRIOV" is detected *and* "amdgpu_mcbp" is not set to "0";
>2) for Bare-metal, we just leave "amdgpu_mcbp" as the value it was....
>3) we interpret  "amdgpu_mcbp"  as:
>0: force disable, it will be "disable" for both BM and SRIOV
>1:  force enable, auto (default), it will be "enable" for both BM and SRIOV
>
>This way if you can disable MCBP in both SRIOV and BM by that existed
>parameter instead of introducing a duplicated one ...
>
>_____________________________________
>Monk Liu|GPU Virtualization Team |AMD
>
>
>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>Emily.Deng
>Sent: Friday, September 18, 2020 11:27 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@amd.com>
>Subject: [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug
>
>For debug convenient, add sriov_mcbp parameter.
>
>Signed-off-by: Emily.Deng <Emily.Deng@amd.com>
>Change-Id: I84019eb4344e00d85b2ecc853145aabb312412fe
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++
>drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 3 ++-
>drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> 4 files changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>index 13f92dea182a..a255fbf4d370 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>@@ -183,6 +183,7 @@ extern uint amdgpu_ras_mask;  extern int
>amdgpu_bad_page_threshold;  extern int amdgpu_async_gfx_ring;  extern int
>amdgpu_mcbp;
>+extern int amdgpu_sriov_mcbp;
> extern int amdgpu_discovery;
> extern int amdgpu_mes;
> extern int amdgpu_noretry;
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>index 3f07d1475bd2..b0b2f0f7be94 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>@@ -145,6 +145,7 @@ uint amdgpu_dc_feature_mask = 0;  uint
>amdgpu_dc_debug_mask = 0;  int amdgpu_async_gfx_ring = 1;  int
>amdgpu_mcbp = 0;
>+int amdgpu_sriov_mcbp = 1;
> int amdgpu_discovery = -1;
> int amdgpu_mes = 0;
> int amdgpu_noretry;
>@@ -578,6 +579,14 @@ MODULE_PARM_DESC(mcbp,  "Enable Mid-command
>buffer preemption (0 = disabled (default), 1 = enabled)");
>module_param_named(mcbp, amdgpu_mcbp, int, 0444);
>
>+/**
>+ * DOC: sriov_mcbp (int)
>+ * It is used to enable mid command buffer preemption. (0 = disabled, 1
>+= enabled(default))  */ MODULE_PARM_DESC(sriov_mcbp, "Enable sriov
>+Mid-command buffer preemption (0 = disabled (default), 1 = enabled)");
>+module_param_named(sriov_mcbp, amdgpu_sriov_mcbp, int, 0444);
>+
> /**
>  * DOC: discovery (int)
>  * Allow driver to discover hardware IP information from IP Discovery table at
>the top of VRAM.
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>index 2f53fa0ae9a6..ca0e17688bdf 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>@@ -236,7 +236,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>unsigned num_ibs,
>
> for (i = 0; i < num_ibs; ++i) {
> ib = &ibs[i];
>-
>+if (!amdgpu_sriov_mcbp)
>+ib->flags &= ~AMDGPU_IB_FLAG_PREEMPT;
> /* drop preamble IBs if we don't have a context switch */  if ((ib->flags &
>AMDGPU_IB_FLAG_PREAMBLE) &&
>     skip_preamble &&
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>index d7f37cb92a97..156e76a5a6e0 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>@@ -742,7 +742,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev,
>void *data, struct drm_file  dev_info.ids_flags = 0;  if (adev->flags &
>AMD_IS_APU)  dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION; -if
>(amdgpu_mcbp || amdgpu_sriov_vf(adev))
>+if (amdgpu_mcbp || (amdgpu_sriov_vf(adev) && amdgpu_sriov_mcbp))
> dev_info.ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;  if
>(amdgpu_is_tmz(adev))  dev_info.ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
>--
>2.25.1
>
>_______________________________________________
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fre
>edesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfx&amp;data=02%7C01%7Cmonk.liu%40amd.com%7C497edf57e53b41a055f
>c08d85b82c709%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
>37359964547689160&amp;sdata=zXlPTG939tdC0sUbCntiJuGsZHpM15DqWmg
>y8SmZ2Z8%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-21  6:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-18  3:27 [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Emily.Deng
2020-09-18  3:27 ` [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank Emily.Deng
2020-09-18  8:20   ` Zhang, Hawking
2020-09-18  8:29     ` Deng, Emily
2020-09-21  4:04 ` [PATCH 1/2] drm/amdgpu/sriov: Add one parameter for mcbp debug Deng, Emily
2020-09-21  5:37 ` Liu, Monk
2020-09-21  6:26   ` Deng, Emily

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.