<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Thanks for verifying. Sent out for review. Alex </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Grodzovsky, Andrey Sent: Monday, January 22, 2018 11:01:36 AM To: Wentland, Harry; Lipski, Mikita; amd-gfx@lists.freedesktop.org; Deucher, Alexander Subject: Re: [PATCH 15/24] drm/amd/display: Fix deadlock when flushing irq <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">NP. Alex just a reminder to push your patch from SWDEV-143068 into the tree. Thanks, Andrey On 01/22/2018 10:53 AM, Harry Wentland wrote: > On 2018-01-22 09:57 AM, Andrey Grodzovsky wrote: >> You guys just need to revert it on dal-dev branch and don't promote to amd-staging-drm-next. >> > K. I'll leave it out of amd-staging-drm-next and we'll revert on our internal branch. > > Thanks for following up on this and good to hear we found a better solution. > > Harry > >> So NAK for the change. >> >> >> >> Thanks, >> >> Andrey >> >> >> On 01/22/2018 09:38 AM, Lipski, Mikita wrote: >>> Hi Andrey, >>> >>> >>> I've checked to revert this change and use Alex's change on switching irq destruction sequence and it worked no problem. >>> You can reject this change if Alex's change is pulled in. >>> >>> >>> Thanks, >>> >>> Nick >>> >>> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ >>> *From:* Grodzovsky, Andrey >>> *Sent:* Friday, January 19, 2018 1:33:51 PM >>> *To:* Wentland, Harry; amd-gfx@lists.freedesktop.org >>> *Cc:* Lipski, Mikita >>> *Subject:* Re: [PATCH 15/24] drm/amd/display: Fix deadlock when flushing irq >>> >>> What this spin lock is protecting here ? Seems to me it's just a read of >>> an array element which is always there. >>> >>> Regarding subsequent remove_timer_handler and timer queue destruction it >>> seems to me to be obsolete code, I don't think DAL is still using the >>> timer queue, >>> >>> so seems to me everything related to it should be removed. >>> >>> Thanks, >>> >>> Andrey >>> >>> >>> On 01/18/2018 04:03 PM, Harry Wentland wrote: >>>> From: Mikita Lipski <mikita.lipski@amd.com> >>>> >>>> Lock irq table when reading a work in queue, >>>> unlock to flush the work, lock again till all tasks >>>> are cleared >>>> >>>> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> >>>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com> >>>> --- >>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c >>>> index 1874b6cee6af..fb60c91a1bfe 100644 >>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c >>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c >>>> @@ -400,14 +400,15 @@ void amdgpu_dm_irq_fini(struct amdgpu_device *adev) >>>> { >>>> int src; >>>> struct irq_list_head *lh; >>>> + unsigned long irq_table_flags; >>>> DRM_DEBUG_KMS("DM_IRQ: releasing resources.\n"); >>>> - >>>> for (src = 0; src < DAL_IRQ_SOURCES_NUMBER; src++) { >>>> - >>>> + DM_IRQ_TABLE_LOCK(adev, irq_table_flags); >>>> /* The handler was removed from the table, >>>> * it means it is safe to flush all the 'work' >>>> * (because no code can schedule a new one). */ >>>> lh = &adev->dm.irq_handler_list_low_tab[src]; >>>> + DM_IRQ_TABLE_UNLOCK(adev, irq_table_flags); >>>> flush_work(&lh->work); >>>> } >>>> </div> </div> </body> </html>