From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752341AbbAaMob (ORCPT ); Sat, 31 Jan 2015 07:44:31 -0500 Received: from gloria.sntech.de ([95.129.55.99]:53624 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751049AbbAaMo3 (ORCPT ); Sat, 31 Jan 2015 07:44:29 -0500 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: dri-devel@lists.freedesktop.org Cc: Mark Yao , David Airlie , Daniel Vetter , Rob Clark , Philipp Zabel , Daniel Kurtz , linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH] drm/rockchip: vop: power off until vop standby take effect Date: Sat, 31 Jan 2015 13:49:28 +0100 Message-ID: <26349423.KqVQYq280G@diego> User-Agent: KMail/4.14.1 (Linux/3.16-3-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <1422693698-21944-1-git-send-email-mark.yao@rock-chips.com> References: <1422693698-21944-1-git-send-email-mark.yao@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mark, Am Samstag, 31. Januar 2015, 16:41:38 schrieb Mark Yao: > Vop standby will take effect end of current frame, > if dsp_hold_valid_irq happen, it means vop standby complete. > > we must wait standby complete when we want to disable aclk, > if not, memory bus maybe dead. > > Signed-off-by: Mark Yao > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 76 > ++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 13 > deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index fb25836..47ea61f 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -89,6 +89,7 @@ struct vop { > /* mutex vsync_ work */ > struct mutex vsync_mutex; > bool vsync_work_pending; > + struct completion dsp_hold_completion; > > const struct vop_data *data; > > @@ -382,6 +383,34 @@ static bool is_alpha_support(uint32_t format) > } > } > > +static void vop_dsp_hold_valid_irq_enable(struct vop *vop) > +{ > + unsigned long flags; > + > + BUG_ON(!vop->is_enabled); BUG_ON is generally not well liked in general error handling - i.e. in the !vop->is_enabled bad things may happen, but this may not always be the case. And BUG_ON effectively kills the machine. You could simply use a WARN_ON, so people see that something really strange is going on but can still try to recover things. > + > + spin_lock_irqsave(&vop->irq_lock, flags); > + > + vop_mask_write(vop, INTR_CTRL0, DSP_HOLD_VALID_INTR_MASK, > + DSP_HOLD_VALID_INTR_EN(1)); > + > + spin_unlock_irqrestore(&vop->irq_lock, flags); > +} > + > +static void vop_dsp_hold_valid_irq_disable(struct vop *vop) > +{ > + unsigned long flags; > + > + BUG_ON(!vop->is_enabled); same here > + > + spin_lock_irqsave(&vop->irq_lock, flags); > + > + vop_mask_write(vop, INTR_CTRL0, DSP_HOLD_VALID_INTR_MASK, > + DSP_HOLD_VALID_INTR_EN(0)); > + > + spin_unlock_irqrestore(&vop->irq_lock, flags); > +} > + > static void vop_enable(struct drm_crtc *crtc) > { > struct vop *vop = to_vop(crtc); except the above Reviewed-by: Heiko Stuebner [but take it with a grain of salt, as I'm still new to drm-land :-) ] Heiko