From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [PATCH] drm/msm/mdp5: enable clocks in hw_init and set_irqmask Date: Fri, 28 Aug 2015 13:26:50 +0530 Message-ID: <55E01442.6040608@codeaurora.org> References: <1440568263-674-1-git-send-email-architt@codeaurora.org> <9475e562be69b5c0ec26c4c63e0f83e0.squirrel@www.codeaurora.org> <0b855d85f2e9ef386b8ed67d89dbcc33.squirrel@www.codeaurora.org> <55DE9ABE.1050409@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:39746 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751074AbbH1H44 (ORCPT ); Fri, 28 Aug 2015 03:56:56 -0400 In-Reply-To: <55DE9ABE.1050409@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: hali@codeaurora.org, Rob Clark Cc: linux-arm-msm , "dri-devel@lists.freedesktop.org" , Stephane Viau On 08/27/2015 10:36 AM, Archit Taneja wrote: > > > On 08/26/2015 08:42 PM, hali@codeaurora.org wrote: >>> 2015-08-26 9:55 GMT-04:00 : >>>> Hi Archit, >>>> >>>>> mdp5_hw_init and mdp5_set_irqmask configure registers but may not have >>>>> clocks enabled. >>>>> >>>>> Add mdp5_enable/disable calls in these funcs to ensure clocks are >>>>> enabled. We need this until we get proper runtime pm support. >>>>> >>>>> Signed-off-by: Archit Taneja >>>>> --- >>>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 10 ++++++++-- >>>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 2 ++ >>>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c >>>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c >>>>> index b1f73be..9fabfca 100644 >>>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c >>>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c >>>>> @@ -24,9 +24,15 @@ >>>>> void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask, >>>>> uint32_t old_irqmask) >>>>> { >>>>> - mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_CLEAR(0), >>>>> + struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms); >>>>> + >>>>> + mdp5_enable(mdp5_kms); >>>>> + >>>>> + mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_CLEAR(0), >>>>> irqmask ^ (irqmask & old_irqmask)); >>>>> - mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_EN(0), >>>>> irqmask); >>>>> + mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_EN(0), irqmask); >>>>> + >>>>> + mdp5_disable(mdp5_kms); >>>>> } >>>> >>>> mdp5_set_irqmask() can be invoked in atomic context, clk_prepare() is >>>> not >>>> allowed in this function because it may cause process to sleep. We can >>>> enable the clocks in the caller at initialization. > > Oh, oops. I missed that. > >>> >>> iirc, it will be called with at least one spinlock held.. >>> >>> We do already move the enable/disable_vblank() paths off to a worker >>> so that we can ensure things are enabled before we get into >>> update_irq().. the only other path to update_irq() should be when >>> driver code does mdp_irq_register/unregister().. so maybe we should >>> just require that the mdp4/mdp5 kms code only calls those when clk's >>> are already enabled (which should be mostly true already, I think) >>> >>> BR, >>> -R >> >> Yes, the only case that not been covered is mdp5_irq_postinstall(). We >> can >> enable clocks in this function. Actually, this is what we are doing in >> downstream test. > > It works fine if I put it in postinstall. I'll update the patch and resend. So, I hit an issue in both the approaches. When I try modeset, I get a watchdog reset once the app closes down. Looking at debug logs, it looks like the issue happens when the ioctl RMFB and drm_release race with each other. Within the the msm driver, this maps to mdp5_complete_commit (drm_mode_rmfb path) being called before mdp5_crtc_cancel_pending_flip (drm_release path). mdp5_complete_commit disables clocks, and the other patch calls complete_flip, which requires clocks. If I wrap around complete_flip with mdp5_enable/disable calls, things work fine. Although, that's not an ideal fix. Any suggestions? Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project