From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0914AC433F5 for ; Thu, 12 May 2022 19:33:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1358052AbiELTdf (ORCPT ); Thu, 12 May 2022 15:33:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350504AbiELTde (ORCPT ); Thu, 12 May 2022 15:33:34 -0400 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F341661627; Thu, 12 May 2022 12:33:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1652384010; x=1683920010; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=aF38eStTDqNFqz4Gx3c+CKSDyxaee9RhF1zN1sw4C8Q=; b=XF3rvfB7DkyN5pmkJuyIX51H2y8IKGCIfdZPKSpHIQKllAvq9mxZYnJ0 JNxq0u8mGwsqEFHuje31PxpNWL+MRdehcwPiKAHbR/35eb8dDy009vnDq Pfy5R9DdwVEp43Jc58ei1pNPizAeE4s/ayg8jVnkihRTgKTcGtRGL1PQc o=; Received: from unknown (HELO ironmsg03-sd.qualcomm.com) ([10.53.140.143]) by alexa-out-sd-01.qualcomm.com with ESMTP; 12 May 2022 12:33:29 -0700 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.47.97.222]) by ironmsg03-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 May 2022 12:33:29 -0700 Received: from nalasex01a.na.qualcomm.com (10.47.209.196) by nasanex01c.na.qualcomm.com (10.47.97.222) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Thu, 12 May 2022 12:33:28 -0700 Received: from [10.110.33.222] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Thu, 12 May 2022 12:33:27 -0700 Message-ID: <45b7c7fd-9e77-a0f8-022b-27e874f4b3f0@quicinc.com> Date: Thu, 12 May 2022 12:33:26 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH v5] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl() Content-Language: en-US To: Dmitry Baryshkov , Stephen Boyd CC: , , , , , , , , , , , , , , References: <1652313768-16286-1-git-send-email-quic_khsieh@quicinc.com> From: Kuogee Hsieh In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nalasex01a.na.qualcomm.com (10.47.209.196) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/11/2022 6:03 PM, Dmitry Baryshkov wrote: > On Thu, 12 May 2022 at 04:01, Stephen Boyd wrote: >> Quoting Dmitry Baryshkov (2022-05-11 17:41:50) >>> On 12/05/2022 03:02, Kuogee Hsieh wrote: >>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c >>>> index af7a80c..f3e333e 100644 >>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c >>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c >>>> @@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable) >>>> >>>> dp_catalog_ctrl_reset(ctrl->catalog); >>>> >>>> - if (enable) >>>> - dp_catalog_ctrl_enable_irq(ctrl->catalog, enable); >>>> + /* >>>> + * all dp controller programmable registers will not >>>> + * be reset to default value after DP_SW_RESET >>>> + * therefore interrupt mask bits have to be updated >>>> + * to enable/disable interrupts >>>> + */ >>>> + dp_catalog_ctrl_enable_irq(ctrl->catalog, enable); >>>> } >>>> >>>> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl) >>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >>>> index c388323..79439b8 100644 >>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c >>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >>>> @@ -98,6 +98,8 @@ struct dp_display_private { >>>> struct dp_ctrl *ctrl; >>>> struct dp_debug *debug; >>>> >>>> + atomic_t suspended; >>> I think it'd be better to protect it with event_lock rather than using >>> atomics. >> Agreed. I think the concern is that the event queue will have "stuff" in >> it. If the event queue was all a threaded irq we could simply call >> synchronize_irq() after disabling the irq bit in the DP hardware and >> then we would know it is safe to power down the DP logic. Unfortunately >> the event queue is a kthread so we can't do that and we have to rewrite >> synchronize_irq() by checking that the event queue is empty and waiting >> for it to empty out otherwise. It's not safe enough to simply do the >> power operations underneath the event_lock because there's a queue in >> the kthread that might be waiting to grab the event_lock to process. > This sounds like a good reason to rewrite event_thread to use > threaded_irq and/or workqueue. I think we are facing two problems, 1) event q is not empty after suspend  (this scenario most likely will not happen since display is off already) -- anyway it should be  fixed by adding "suspended" flag checking 2) new events add after suspend  due to irq mask bits were not cleared (this scenario most likely the major culprit) -- this fixed by remove "enable" flag check at  dp_ctrl_reset_irq_ctrl(). I will have "suspended" flag protected by event_lock. >