From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932818AbeAXJYu (ORCPT ); Wed, 24 Jan 2018 04:24:50 -0500 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:36620 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932512AbeAXJYr (ORCPT ); Wed, 24 Jan 2018 04:24:47 -0500 From: Philippe CORNU To: Brian Norris CC: Archit Taneja , Andrzej Hajda , Laurent Pinchart , David Airlie , Benjamin Gaignard , Bhumika Goyal , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , Sandy Huang , Heiko Stubner , "linux-arm-kernel@lists.infradead.org" , "linux-rockchip@lists.infradead.org" , Yannick FERTRE , Vincent ABRIOU , Alexandre TORGUE , Maxime Coquelin , Ludovic BARRE , Mickael REULIER , "hl@rock-chips.com" , "Chris Zhong" , "nickey.yang@rock-chips.com" Subject: Re: [PATCH v3] drm/bridge/synopsys: dsi: add optional pixel clock Thread-Topic: [PATCH v3] drm/bridge/synopsys: dsi: add optional pixel clock Thread-Index: AQHTlGzLGXXRIzYyCUCpCWgoURDUQKOB3cMAgADSxwA= Date: Wed, 24 Jan 2018 09:24:06 +0000 Message-ID: <507552a3-c68f-d6ac-a747-b8f5168d0d41@st.com> References: <20180123170806.5282-1-philippe.cornu@st.com> <20180123204939.qirvr34mlwgiwmqz@ban.mtv.corp.google.com> In-Reply-To: <20180123204939.qirvr34mlwgiwmqz@ban.mtv.corp.google.com> Accept-Language: fr-FR, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.75.127.45] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2018-01-24_04:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id w0O9OuKE000712 Hi Brian, On 01/23/2018 09:49 PM, Brian Norris wrote: > Hi, > > Philippe asked me to review the last version. I'm not sure I have a lot > to contribute. Maybe Rockchip folks who wrote this stuff in the first > place might. I've CC'd some. > > On Tue, Jan 23, 2018 at 06:08:06PM +0100, Philippe Cornu wrote: >> The pixel clock is optional. When available, it offers a better >> preciseness for timing computations and allows to reduce the extra dsi >> bandwidth in burst mode (from ~20% to ~10-12%, hw platform dependant). >> >> Reviewed-by: Andrzej Hajda >> Signed-off-by: Philippe Cornu >> --- >> Changes in v3: Simplify px_clk probing thanks to Andrzej Hajda comments >> >> Changes in v2: Improve px_clk probing in case of ENOENT dt returned value >> (thanks to Philipp Zabel & Andrzej Hajda comments) >> >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 25 +++++++++++++++++++------ >> 1 file changed, 19 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> index ed8af32f8e52..9fb35385c348 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> @@ -217,6 +217,7 @@ struct dw_mipi_dsi { >> void __iomem *base; >> >> struct clk *pclk; >> + struct clk *px_clk; >> >> unsigned int lane_mbps; /* per lane */ >> u32 channel; >> @@ -703,24 +704,28 @@ static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge, >> struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); >> const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; >> void *priv_data = dsi->plat_data->priv_data; >> + struct drm_display_mode px_clk_mode = *mode; >> int ret; >> >> clk_prepare_enable(dsi->pclk); >> >> - ret = phy_ops->get_lane_mbps(priv_data, mode, dsi->mode_flags, >> + if (dsi->px_clk) >> + px_clk_mode.clock = clk_get_rate(dsi->px_clk) / 1000; >> + >> + ret = phy_ops->get_lane_mbps(priv_data, &px_clk_mode, dsi->mode_flags, >> dsi->lanes, dsi->format, &dsi->lane_mbps); >> if (ret) >> DRM_DEBUG_DRIVER("Phy get_lane_mbps() failed\n"); >> >> pm_runtime_get_sync(dsi->dev); >> dw_mipi_dsi_init(dsi); >> - dw_mipi_dsi_dpi_config(dsi, mode); >> + dw_mipi_dsi_dpi_config(dsi, &px_clk_mode); >> dw_mipi_dsi_packet_handler_config(dsi); >> dw_mipi_dsi_video_mode_config(dsi); >> - dw_mipi_dsi_video_packet_config(dsi, mode); >> + dw_mipi_dsi_video_packet_config(dsi, &px_clk_mode); >> dw_mipi_dsi_command_mode_config(dsi); >> - dw_mipi_dsi_line_timer_config(dsi, mode); >> - dw_mipi_dsi_vertical_timing_config(dsi, mode); >> + dw_mipi_dsi_line_timer_config(dsi, &px_clk_mode); >> + dw_mipi_dsi_vertical_timing_config(dsi, &px_clk_mode); >> >> dw_mipi_dsi_dphy_init(dsi); >> dw_mipi_dsi_dphy_timing_config(dsi); >> @@ -734,7 +739,7 @@ static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge, >> >> dw_mipi_dsi_dphy_enable(dsi); >> >> - dw_mipi_dsi_wait_for_two_frames(mode); >> + dw_mipi_dsi_wait_for_two_frames(&px_clk_mode); >> >> /* Switch to cmd mode for panel-bridge pre_enable & panel prepare */ >> dw_mipi_dsi_set_mode(dsi, 0); >> @@ -828,6 +833,14 @@ __dw_mipi_dsi_probe(struct platform_device *pdev, >> return ERR_PTR(ret); >> } >> >> + dsi->px_clk = devm_clk_get(dev, "px_clk"); > > Did you write a device tree binding document update for this anywhere? > > Brian > Many thanks for your review, yes, "px_clk" is already documented, please have a look to Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt Philippe :-) >> + if (IS_ERR(dsi->px_clk)) { >> + ret = PTR_ERR(dsi->px_clk); >> + if (ret != ENOENT) >> + dev_err(dev, "Unable to get opt. px_clk: %d\n", ret); >> + dsi->px_clk = NULL; >> + } >> + >> /* >> * Note that the reset was not defined in the initial device tree, so >> * we have to be prepared for it not being found. >> -- >> 2.15.1 >>