From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752091AbdJ0GlO (ORCPT ); Fri, 27 Oct 2017 02:41:14 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:46201 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750828AbdJ0GlM (ORCPT ); Fri, 27 Oct 2017 02:41:12 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20171027064109euoutp0191f77c5061379dbaad3649254efb4cd3~xWiXtH11q1643816438euoutp01U X-AuditID: cbfec7f4-f79ab6d000003290-e2-59f2d5045c3a MIME-version: 1.0 Content-type: text/plain; charset="utf-8" Subject: Re: [PATCH] drm/bridge/synopsys: dsi: add optional pixel clock To: Philippe Cornu , Archit Taneja , Laurent Pinchart , David Airlie , Philipp Zabel , Benjamin Gaignard , Bhumika Goyal , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: Yannick Fertre , Vincent Abriou , Alexandre Torgue , Maxime Coquelin , Gabriel Fernandez , Ludovic Barre , Fabien Dessenne , Mickael Reulier From: Andrzej Hajda Message-id: <9e4b9bd7-8728-49c4-e0e1-91c68b8eac7c@samsung.com> Date: Fri, 27 Oct 2017 08:41:05 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 In-reply-to: <1509034175-23767-1-git-send-email-philippe.cornu@st.com> Content-transfer-encoding: 8bit Content-language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA01Sa0hTYRjmO7cdV4vTsny7kDEqMLpoaH10syDrFHT5E6RQOfKgkTPbUcl+ iFmzuWGaCuq0EMpbVGIXzSiaKzZrooUXFGspWC0xXHNWSrrczoL9ey/P8z3v8/CxpHKUXsGe S8sQtGnqVBUjp1qs012bqD53fOSrBhoXdr0lcPOoHeE8/Q8a51neMdjtKGJw79QEg60PWxG+ 9rmTwAU378pwz/NqBr8x78O3Zx/Q+F27hcCfHB0UHr7mpHFnn5nC1wt0zN7FfM+NQoJvM32S 8VX6Spr/2P+C4Vt/DdP8Z6ON4ItmI/kvf16S/OSj1cdDEuS7koTUc1mCdsueRHmKd2JIlt6y 8pLLM07kot5lBhTCAhcN3xqGZFK9DN47mhgDkrNKrhZBta6UkJpJBGPtNcR/xlW7OYCqQ9BU +JT2LRTcYvhT6qB8NclFgNNTQkmgrwj+zupIA2LZJdxBGOnyz0M5FwF1Xqf/JZIbIqDNUcb4 2Mw8e/bxIOMjKLg90H/lsG9Mcetg2Gjwiy3lToLtdT3y1SHcAWi0mBlJOBzae78FjgiDq7pB vxhwz2UwcH+Ckizsh7bB7oCdJTBmexIIYBX0lBoDBCMCd1GHTGrKEMy5KkgJtRNe2z7QksQi KGkp91sDTgH6fKUE4cHprA4I7IM+17hMiqISwWRdL1GMwk1BkZmCIjMFuTAFuahB1D0UKmSK mmRBjN4sqjViZlry5rMXNI/Q/N+zz9k8z9Ad6w4L4likWqhoiHDHK2l1lpitsSBgSVWoYqt1 fqRIUmdfFrQXzmgzUwXRglaylCpMsTshP17JJaszhPOCkC5o/28JNmRFLso3jv8mxsvsKPLL mQ5Z6cZbkzGx6c1s97Zmt7689ueq+z05cTnalONV3Ejz6ek1EfXOqVPL7UmNSTMNfExiGBnX eX2hZ/Bld/SAWFKxbYE7oii8seME+fPQguK+rpHcqJy5UXrtd5Pp2HZX8Vhs9fqm7raslvSj aYkzlDf1iPeiihJT1FEbSK2o/gfYL9gpdwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrGIsWRmVeSWpSXmKPExsVy+t/xy7rMVz9FGhw9LWHRe+4kk8XGJ6cZ LZo63rJaNB06xWbx6V4/m8WVr+/ZLI6t285o0XL/DJNF58Ql7BaXd81hszhywNFi3t+1rBan Dh5isrh77wSLxYOWF6wWZ64eYLFo72xlcxD0uNzXy+Sxc9Zddo/ZHTNZPe5c28Pmsf3bA1aP +93HmTz6/xp4PP2xl9nj8ya5AM4oLpuU1JzMstQifbsEroz/72+zF2yTrvjw5Q1TA+MVsS5G Tg4JAROJ5tMH2CBsMYkL99YD2VwcQgJLGCUuTJrEDJLgFRCU+DH5HksXIwcHs4C6xJQpuRA1 z4Bq7m8FiwsLuEs8PMcCEhcR+MQk0fmoF8xhFnjIJNH37gUzRMdsRokPu2cygkxlE9CU+Lv5 JhtIN6+AncS1Ri+QMIuAqsSD7i5WEFtUIELiefN7MJtTwE1i5SGIS5kF5CUOXnnOAmGLSzS3 3mSZwCg4C8mtsxBunYWkYxaSjgWMLKsYRVJLi3PTc4uN9IoTc4tL89L1kvNzNzECY3PbsZ9b djB2vQs+xCjAwajEw7tC81OkEGtiWXFl7iFGCQ5mJRFeo2NAId6UxMqq1KL8+KLSnNTiQ4zS HCxK4ry9e1ZHCgmkJ5akZqemFqQWwWSZODilGhgvXosufS91xVlc6UDESfOtH0WWGvye4bR5 h1/ijbrHN9c9W7Vn3+x5kzOnv+ZaIfCmKi4k7l6qdC1v/QeB+66nLsfW7V4wYbXO5r37Wxm9 pt5P7Dk//aukxZZCHonXL7jWV+bdDLixSJSpbP1KWVm1ewEXa5YGmtubqCVzZvEFhYjO/HKt +aeVEktxRqKhFnNRcSIAQXx8HMkCAAA= X-CMS-MailID: 20171027064107eucas1p237e361624663595876653623631f9e3f X-Msg-Generator: CA CMS-TYPE: 201P X-CMS-RootMailID: 20171026161013epcas2p2a127f113bf4db8f2922bbf4b0874e8b5 X-RootMTR: 20171026161013epcas2p2a127f113bf4db8f2922bbf4b0874e8b5 References: <1509034175-23767-1-git-send-email-philippe.cornu@st.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26.10.2017 18:09, Philippe Cornu wrote: > The pixel clock is optional. When available, it offers a better > preciseness for timing computations. > > Signed-off-by: Philippe Cornu > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 24 ++++++++++++++++++------ > 1 file changed, 18 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 d9cca4f..8b3787d 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -225,6 +225,7 @@ struct dw_mipi_dsi { > void __iomem *base; > > struct clk *pclk; > + struct clk *px_clk; > > unsigned int lane_mbps; /* per lane */ > u32 channel; > @@ -753,24 +754,28 @@ 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); Just small suggestion: if pixel clock rate matters, maybe better is to fix it in adjusted_mode in mode_fixup callback. > 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); > @@ -784,7 +789,7 @@ 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); > @@ -878,6 +883,13 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) > return ERR_PTR(ret); > } > > + dsi->px_clk = devm_clk_get(dev, "px_clk"); > + if (IS_ERR(dsi->px_clk)) { > + ret = PTR_ERR(dsi->px_clk); > + dev_dbg(dev, "Unable to get optional px_clk: %d\n", ret); > + dsi->px_clk = NULL; > + } > + devm_clk_get is called from bridge::attach callback, do we have guarantee that in ::detach callback the clock will be removed? And what if the clock is defined in dts, but it cannot be get due to other reasons? I guess the code should fail then, ie you should have different paths for -ENOENT and for other errors.  -- Regards Andrzej > /* > * Note that the reset was not defined in the initial device tree, so > * we have to be prepared for it not being found.