From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752554AbcGUO2c (ORCPT ); Thu, 21 Jul 2016 10:28:32 -0400 Received: from mail-yw0-f174.google.com ([209.85.161.174]:33912 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751726AbcGUO2a (ORCPT ); Thu, 21 Jul 2016 10:28:30 -0400 MIME-Version: 1.0 In-Reply-To: <1469106886-4979-1-git-send-email-ykk@rock-chips.com> References: <1469106874-4912-1-git-send-email-ykk@rock-chips.com> <1469106886-4979-1-git-send-email-ykk@rock-chips.com> From: Sean Paul Date: Thu, 21 Jul 2016 10:28:06 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1 2/2] drm/bridge: analogix_dp: turn off the panel when eDP need to disable To: Yakir Yang Cc: Mark Yao , Inki Dae , Thierry Reding , Heiko Stuebner , Jingoo Han , Javier Martinez Canillas , =?UTF-8?Q?St=C3=A9phane_Marchesin?= , Tomasz Figa , Douglas Anderson , David Airlie , Daniel Vetter , Krzysztof Kozlowski , Emil Velikov , 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 On Thu, Jul 21, 2016 at 9:14 AM, Yakir Yang wrote: > Some panels (like Sharp LQ123P1JX31) need to be turn off when eDP > controller stop to send valid video signal, otherwhise panel would > go burn in, and keep flicker and flicker. > > So it's better to turn off the panel when eDP need to disable, and > we need to turn on the panel in connector->detect() callback, so > that driver would detect panel status rightly. > > Signed-off-by: Yakir Yang > --- > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 32715da..ea059b3 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -961,6 +961,14 @@ analogix_dp_detect(struct drm_connector *connector, bool force) > { > struct analogix_dp_device *dp = to_dp(connector); > > + /* > + * Panle would prepare for several times here, but don't worry it s/Panle/Panel/ > + * would only enable the hardware at the first prepare time. Errr, this shouldn't go in detect. How about putting this in bridge_enable instead? > + */ > + if (dp->plat_data->panel) > + if (drm_panel_prepare(dp->plat_data->panel)) Personally, I don't like doing work in a conditional since you're throwing the return code away. Could you break this out into: ret = drm_panel_prepare(dp->plat_data->panel); if (ret) DRM_ERROR("failed to setup the panel ret=%d\n", ret); > + DRM_ERROR("failed to setup the panel\n"); > + > if (analogix_dp_detect_hpd(dp)) > return connector_status_disconnected; > > @@ -1063,7 +1071,8 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) > return; > > if (dp->plat_data->panel) { > - if (drm_panel_disable(dp->plat_data->panel)) { > + if (drm_panel_disable(dp->plat_data->panel) || > + drm_panel_unprepare(dp->plat_data->panel)) { Same comment here, please break this out into separate statements for better readability/logging. > DRM_ERROR("failed to disable the panel\n"); > return; > } > @@ -1333,13 +1342,6 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, > > phy_power_on(dp->phy); > > - if (dp->plat_data->panel) { > - if (drm_panel_prepare(dp->plat_data->panel)) { > - DRM_ERROR("failed to setup the panel\n"); > - return -EBUSY; > - } > - } > - > analogix_dp_init_dp(dp); > > ret = devm_request_threaded_irq(&pdev->dev, dp->irq, > -- > 1.9.1 > >