You guys just need to revert it on dal-dev branch and don't promote to amd-staging-drm-next.

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; [email protected]
*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 <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Harry Wentland <[email protected]>
> ---
> 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);
>        }
>


_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to