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: Thu, 27 Aug 2015 10:36:06 +0530 Message-ID: <55DE9ABE.1050409@codeaurora.org> References: <1440568263-674-1-git-send-email-architt@codeaurora.org> <9475e562be69b5c0ec26c4c63e0f83e0.squirrel@www.codeaurora.org> <0b855d85f2e9ef386b8ed67d89dbcc33.squirrel@www.codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:44159 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751612AbbH0FGL (ORCPT ); Thu, 27 Aug 2015 01:06:11 -0400 In-Reply-To: <0b855d85f2e9ef386b8ed67d89dbcc33.squirrel@www.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: "dri-devel@lists.freedesktop.org" , linux-arm-msm , Stephane Viau 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. Thanks, Archit >> >>>> >>>> static void mdp5_irq_error_handler(struct mdp_irq *irq, uint32_t >>>> irqstatus) >>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c >>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c >>>> index 047cb04..2b760f5 100644 >>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c >>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c >>>> @@ -32,6 +32,7 @@ static int mdp5_hw_init(struct msm_kms *kms) >>>> unsigned long flags; >>>> >>>> pm_runtime_get_sync(dev->dev); >>>> + mdp5_enable(mdp5_kms); >>>> >>>> /* Magic unknown register writes: >>>> * >>>> @@ -63,6 +64,7 @@ static int mdp5_hw_init(struct msm_kms *kms) >>>> >>>> mdp5_ctlm_hw_reset(mdp5_kms->ctlm); >>>> >>>> + mdp5_disable(mdp5_kms); >>>> pm_runtime_put_sync(dev->dev); >>>> >>>> return 0; >>>> -- >>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >>>> Forum, >>>> hosted by The Linux Foundation >>>> >>>> >>> >>> >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project