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

2020-09-18 Thread Deng, Emily
Thanks, will double check.

Best wishes
Emily Deng



>-Original Message-
>From: Zhang, Hawking 
>Sent: Friday, September 18, 2020 4:20 PM
>To: Deng, Emily ; amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily 
>Subject: RE: [PATCH 2/2] drm/amdgpu: Fix dead lock issue for vblank
>
>[AMD Public Use]
>
>+  spin_lock_irqsave(>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  On Behalf Of
>Emily.Deng
>Sent: Friday, September 18, 2020 11:27
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily 
>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 
>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(>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(>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(_crtc->base, works-
>>event);
>+
>+  spin_unlock_irqrestore(>ddev->event_lock, flags);
>+
>+  drm_crtc_vblank_put(_crtc->base);
>+  amdgpu_bo_unref(>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_in

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

2020-09-18 Thread Zhang, Hawking
[AMD Public Use]

+   spin_lock_irqsave(>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  On Behalf Of Emily.Deng
Sent: Friday, September 18, 2020 11:27
To: amd-gfx@lists.freedesktop.org
Cc: Deng, Emily 
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 
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(>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(>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(_crtc->base, works->event);
+
+   spin_unlock_irqrestore(>ddev->event_lock, flags);
+
+   drm_crtc_vblank_put(_crtc->base);
+   amdgpu_bo_unref(>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(_crtc->base, _virtual_crtc_helper_funcs);
 
+   hrtimer_init(_crtc->vblank_timer,
+CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+   hrtimer_set_expires(_crtc->vblank_timer,
+   ktime_set(0, DCE_VIRTUAL_VBLANK_PERIOD));
+   amdgpu_crtc->vblank_timer.function =
+   dce_virtual_vblank_timer_handle;
+   hrtimer_start(_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; imode_info.num_crtc; i++)
if (adev->mode_info.crtcs[i])
-