Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936366AbcJRPwr (ORCPT ); Tue, 18 Oct 2016 11:52:47 -0400 Received: from mail-yw0-f182.google.com ([209.85.161.182]:35624 "EHLO mail-yw0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934237AbcJRPwl (ORCPT ); Tue, 18 Oct 2016 11:52:41 -0400 MIME-Version: 1.0 In-Reply-To: <1476771283-11398-2-git-send-email-wzz@rock-chips.com> References: <1476771283-11398-1-git-send-email-wzz@rock-chips.com> <1476771283-11398-2-git-send-email-wzz@rock-chips.com> From: Sean Paul Date: Tue, 18 Oct 2016 11:52:16 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 1/6] drm/bridge: analogix_dp: set psr activate/deactivate when enable/disable bridge To: Zain Wang , Archit Cc: Daniel Vetter , Inki Dae , David Airlie , Tomeu Vizoso , Mika Kahola , =?UTF-8?Q?St=C3=A9phane_Marchesin?= , Tomasz Figa , Doug Anderson , Thierry Reding , Krzysztof Kozlowski , Heiko Stuebner , Jingoo Han , Javier Martinez Canillas , Linux Kernel Mailing List , dri-devel , linux-samsung-soc , linux-rockchip@lists.infradead.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5724 Lines: 156 On Tue, Oct 18, 2016 at 2:14 AM, Zain Wang wrote: > From: zain wang > > There's a race between when bridge_disable and when vop_crtc_disable are called. > If the flush timer triggers a new psr work between these, we will operate eDP > without power shutdowned by bridge_disable. > In this case, moving activate/deactivate to enable/disable bridge to avoid it. > Reviewed-by: Sean Paul > Signed-off-by: zain wang > --- > > Changes in v3: > - remove changes before. > - move psr activat/deactivate to enable/disable bridge. > > Changes in v2: > - add spin_lock to protect dpms_mode > > > drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 4 ++-- > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 30 ++++++++++++++++++++----- > drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 4 ++-- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ---- > 4 files changed, 28 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > index 8548e82..e5471e7 100644 > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > @@ -159,7 +159,7 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data) > return ret; > } > > - return 0; > + return rockchip_drm_psr_activate(&dp->encoder); > } > > static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data) > @@ -168,7 +168,7 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data) > > clk_disable_unprepare(dp->pclk); > > - return 0; > + return rockchip_drm_psr_deactivate(&dp->encoder); > } > > static int rockchip_dp_get_modes(struct analogix_dp_plat_data *plat_data, > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > index a553e18..4c379e9 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > @@ -57,6 +57,24 @@ out: > return psr; > } > > +static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) > +{ > + struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; > + struct psr_drv *psr; > + unsigned long flags; > + > + spin_lock_irqsave(&drm_drv->psr_list_lock, flags); > + list_for_each_entry(psr, &drm_drv->psr_list, list) { > + if (psr->encoder == encoder) > + goto out; > + } > + psr = ERR_PTR(-ENODEV); > + > +out: > + spin_unlock_irqrestore(&drm_drv->psr_list_lock, flags); > + return psr; > +} > + > static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) > { > /* > @@ -115,14 +133,14 @@ static void psr_flush_handler(unsigned long data) > > /** > * rockchip_drm_psr_activate - activate PSR on the given pipe > - * @crtc: CRTC to obtain the PSR encoder > + * @encoder: encoder to obtain the PSR encoder > * > * Returns: > * Zero on success, negative errno on failure. > */ > -int rockchip_drm_psr_activate(struct drm_crtc *crtc) > +int rockchip_drm_psr_activate(struct drm_encoder *encoder) > { > - struct psr_drv *psr = find_psr_by_crtc(crtc); > + struct psr_drv *psr = find_psr_by_encoder(encoder); > unsigned long flags; > > if (IS_ERR(psr)) > @@ -138,14 +156,14 @@ EXPORT_SYMBOL(rockchip_drm_psr_activate); > > /** > * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe > - * @crtc: CRTC to obtain the PSR encoder > + * @encoder: encoder to obtain the PSR encoder > * > * Returns: > * Zero on success, negative errno on failure. > */ > -int rockchip_drm_psr_deactivate(struct drm_crtc *crtc) > +int rockchip_drm_psr_deactivate(struct drm_encoder *encoder) > { > - struct psr_drv *psr = find_psr_by_crtc(crtc); > + struct psr_drv *psr = find_psr_by_encoder(encoder); > unsigned long flags; > > if (IS_ERR(psr)) > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > index b420cf1..b1ea015 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > @@ -18,8 +18,8 @@ > void rockchip_drm_psr_flush_all(struct drm_device *dev); > int rockchip_drm_psr_flush(struct drm_crtc *crtc); > > -int rockchip_drm_psr_activate(struct drm_crtc *crtc); > -int rockchip_drm_psr_deactivate(struct drm_crtc *crtc); > +int rockchip_drm_psr_activate(struct drm_encoder *encoder); > +int rockchip_drm_psr_deactivate(struct drm_encoder *encoder); > > int rockchip_drm_psr_register(struct drm_encoder *encoder, > void (*psr_set)(struct drm_encoder *, bool enable)); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index c7eba30..1740a0b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -566,8 +566,6 @@ static void vop_crtc_disable(struct drm_crtc *crtc) > > WARN_ON(vop->event); > > - rockchip_drm_psr_deactivate(&vop->crtc); > - > /* > * We need to make sure that all windows are disabled before we > * disable that crtc. Otherwise we might try to scan from a destroyed > @@ -975,8 +973,6 @@ static void vop_crtc_enable(struct drm_crtc *crtc) > clk_set_rate(vop->dclk, adjusted_mode->clock * 1000); > > VOP_CTRL_SET(vop, standby, 0); > - > - rockchip_drm_psr_activate(&vop->crtc); > } > > static bool vop_fs_irq_is_pending(struct vop *vop) > -- > 1.9.1 > >