<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, "EmojiFont", "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr"> <div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, "EmojiFont", "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr"> <div> Hi Andrey, I've checked to revert this change and use Alex's change on switching irq destruction sequence and it worked no problem. </div> You can reject this change if Alex's change is pulled in. Thanks, Nick </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">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 <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">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> </div> </body> </html>