From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH] drm/bridge/synopsys: dsi: use adjusted_mode in mode_set Date: Mon, 29 Jan 2018 11:46:35 +0200 Message-ID: <2782155.lsFkNecG25@avalon> References: <20180125155504.8611-1-philippe.cornu@st.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180125155504.8611-1-philippe.cornu@st.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Philippe Cornu Cc: Maxime Coquelin , linux-rockchip@lists.infradead.org, David Airlie , daniel.vetter@ffwll.ch, Brian Norris , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Yannick Fertre , linux-arm-kernel@lists.infradead.org, Ludovic Barre , Mickael Reulier , Vincent Abriou , Bhumika Goyal , Alexandre Torgue List-Id: linux-rockchip.vger.kernel.org SGkgUGhpbGlwcGUsCgooQ0MnaW5nIERhbmllbCBWZXR0ZXIpCgpUaGFuayB5b3UgZm9yIHRoZSBw YXRjaC4KCk9uIFRodXJzZGF5LCAyNSBKYW51YXJ5IDIwMTggMTc6NTU6MDQgRUVUIFBoaWxpcHBl IENvcm51IHdyb3RlOgo+IFRoZSAiYWRqdXN0ZWRfbW9kZSIgY2xvY2sgdmFsdWUgKGllIHRoZSBy ZWFsIHBpeGVsIGNsb2NrKSBpcyBtb3JlCj4gYWNjdXJhdGUgdGhhbiAibW9kZSIgY2xvY2sgdmFs dWUgKGllIHRoZSBwYW5lbC9icmlkZ2UgcmVxdWVzdGVkCj4gY2xvY2sgdmFsdWUpLiBJdCBvZmZl cnMgYSBiZXR0ZXIgcHJlY2lzZW5lc3MgZm9yIHRpbWluZwo+IGNvbXB1dGF0aW9ucyBhbmQgYWxs b3dzIHRvIHJlZHVjZSB0aGUgZXh0cmEgZHNpIGJhbmR3aWR0aCBpbgo+IGJ1cnN0IG1vZGUgKGZy b20gfjIwJSB0byB+MTAtMTIlLCBodyBwbGF0Zm9ybSBkZXBlbmRhbnQpLgo+IAo+IFNpZ25lZC1v ZmYtYnk6IFBoaWxpcHBlIENvcm51IDxwaGlsaXBwZS5jb3JudUBzdC5jb20+CgpUaGUgYWRqdXN0 ZWQgbW9kZSBpcyBkb2N1bWVudGVkIGFzCgogICAgLyoqCiAgICAgKiBAYWRqdXN0ZWRfbW9kZToK ICAgICAqCiAgICAgKiBJbnRlcm5hbCBkaXNwbGF5IHRpbWluZ3Mgd2hpY2ggY2FuIGJlIHVzZWQg YnkgdGhlIGRyaXZlciB0byBoYW5kbGUKICAgICAqIGRpZmZlcmVuY2VzIGJldHdlZW4gdGhlIG1v ZGUgcmVxdWVzdGVkIGJ5IHVzZXJzcGFjZSBpbiBAbW9kZSBhbmQgd2hhdAogICAgICogaXMgYWN0 dWFsbHkgcHJvZ3JhbW1lZCBpbnRvIHRoZSBoYXJkd2FyZS4gSXQgaXMgcHVyZWx5IGRyaXZlcgog ICAgICogaW1wbGVtZW50YXRpb24gZGVmaW5lZCB3aGF0IGV4YWN0bHkgdGhpcyBhZGp1c3RlZCBt b2RlIG1lYW5zLiBVc3VhbGx5CiAgICAgKiBpdCBpcyB1c2VkIHRvIHN0b3JlIHRoZSBoYXJkd2Fy ZSBkaXNwbGF5IHRpbWluZ3MgdXNlZCBiZXR3ZWVuIHRoZQogICAgICogQ1JUQyBhbmQgZW5jb2Rl ciBibG9ja3MuCiAgICAgKi8KClRoaXMgaXMgZWFzeSB0byBoYW5kbGUgd2hlbiB0aGUgQ1JUQyBh bmQgZW5jb2RlciBhcmUgY29udHJvbGxlZCBieSB0aGUgc2FtZSAKZHJpdmVyLCBhcyB0aGUgZmll bGQgaXMgImltcGxlbWVudGF0aW9uIGRlZmluZWQiIGJ5IGEgc2luZ2xlIGRyaXZlciAuIEhvd2V2 ZXIsIAp3aGVuIHVzaW5nIGJyaWRnZXMsIHRoZXJlIGFyZSB0d28gZHJpdmVycyBpbnZvbHZlZCwg YW5kIHRoZXkgbXVzdCBib3RoIGFncmVlIAp0byBtZWFuaW5nZnVsbHkgdXNlIHRoZSBhZGp1c3Rl ZCBtb2RlLiBJIGNhbid0IHNlZSBob3cgdG8gZG8gc28gd2l0aG91dCAKc3RhbmRhcmRpemluZyB0 aGUgbWVhbmluZyBvZiB0aGUgYWRqdXN0ZWQgbW9kZSBmaWVsZC4KCkRhbmllbCwgd2hhdCdzIHlv dXIgb3BpbmlvbiBvbiB0aGlzID8KCj4gLS0tCj4gTm90ZTogVGhpcyBwYXRjaCByZXBsYWNlcyAi ZHJtL2JyaWRnZS9zeW5vcHN5czogZHNpOiBhZGQgb3B0aW9uYWwgcGl4ZWwKPiBjbG9jayIKPiAK PiAgZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1taXBpLWRzaS5jIHwgMTIgKysr KysrLS0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA2IGluc2VydGlvbnMoKyksIDYgZGVsZXRpb25z KC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2Uvc3lub3BzeXMvZHct bWlwaS1kc2kuYwo+IGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1taXBpLWRz aS5jIGluZGV4Cj4gZWQ4YWYzMmY4ZTUyLi5iOTI2YjYyZTllMzMgMTAwNjQ0Cj4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1taXBpLWRzaS5jCj4gKysrIGIvZHJpdmVy cy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1taXBpLWRzaS5jCj4gQEAgLTcwNywyMCArNzA3 LDIwIEBAIHN0YXRpYyB2b2lkIGR3X21pcGlfZHNpX2JyaWRnZV9tb2RlX3NldChzdHJ1Y3QKPiBk cm1fYnJpZGdlICpicmlkZ2UsCj4gCj4gIAljbGtfcHJlcGFyZV9lbmFibGUoZHNpLT5wY2xrKTsK PiAKPiAtCXJldCA9IHBoeV9vcHMtPmdldF9sYW5lX21icHMocHJpdl9kYXRhLCBtb2RlLCBkc2kt Pm1vZGVfZmxhZ3MsCj4gKwlyZXQgPSBwaHlfb3BzLT5nZXRfbGFuZV9tYnBzKHByaXZfZGF0YSwg YWRqdXN0ZWRfbW9kZSwgZHNpLT5tb2RlX2ZsYWdzLAo+ICAJCQkJICAgICBkc2ktPmxhbmVzLCBk c2ktPmZvcm1hdCwgJmRzaS0+bGFuZV9tYnBzKTsKPiAgCWlmIChyZXQpCj4gIAkJRFJNX0RFQlVH X0RSSVZFUigiUGh5IGdldF9sYW5lX21icHMoKSBmYWlsZWRcbiIpOwo+IAo+ICAJcG1fcnVudGlt ZV9nZXRfc3luYyhkc2ktPmRldik7Cj4gIAlkd19taXBpX2RzaV9pbml0KGRzaSk7Cj4gLQlkd19t aXBpX2RzaV9kcGlfY29uZmlnKGRzaSwgbW9kZSk7Cj4gKwlkd19taXBpX2RzaV9kcGlfY29uZmln KGRzaSwgYWRqdXN0ZWRfbW9kZSk7Cj4gIAlkd19taXBpX2RzaV9wYWNrZXRfaGFuZGxlcl9jb25m aWcoZHNpKTsKPiAgCWR3X21pcGlfZHNpX3ZpZGVvX21vZGVfY29uZmlnKGRzaSk7Cj4gLQlkd19t aXBpX2RzaV92aWRlb19wYWNrZXRfY29uZmlnKGRzaSwgbW9kZSk7Cj4gKwlkd19taXBpX2RzaV92 aWRlb19wYWNrZXRfY29uZmlnKGRzaSwgYWRqdXN0ZWRfbW9kZSk7Cj4gIAlkd19taXBpX2RzaV9j b21tYW5kX21vZGVfY29uZmlnKGRzaSk7Cj4gLQlkd19taXBpX2RzaV9saW5lX3RpbWVyX2NvbmZp Zyhkc2ksIG1vZGUpOwo+IC0JZHdfbWlwaV9kc2lfdmVydGljYWxfdGltaW5nX2NvbmZpZyhkc2ks IG1vZGUpOwo+ICsJZHdfbWlwaV9kc2lfbGluZV90aW1lcl9jb25maWcoZHNpLCBhZGp1c3RlZF9t b2RlKTsKPiArCWR3X21pcGlfZHNpX3ZlcnRpY2FsX3RpbWluZ19jb25maWcoZHNpLCBhZGp1c3Rl ZF9tb2RlKTsKPiAKPiAgCWR3X21pcGlfZHNpX2RwaHlfaW5pdChkc2kpOwo+ICAJZHdfbWlwaV9k c2lfZHBoeV90aW1pbmdfY29uZmlnKGRzaSk7Cj4gQEAgLTczNCw3ICs3MzQsNyBAQCBzdGF0aWMg dm9pZCBkd19taXBpX2RzaV9icmlkZ2VfbW9kZV9zZXQoc3RydWN0Cj4gZHJtX2JyaWRnZSAqYnJp ZGdlLAo+IAo+ICAJZHdfbWlwaV9kc2lfZHBoeV9lbmFibGUoZHNpKTsKPiAKPiAtCWR3X21pcGlf ZHNpX3dhaXRfZm9yX3R3b19mcmFtZXMobW9kZSk7Cj4gKwlkd19taXBpX2RzaV93YWl0X2Zvcl90 d29fZnJhbWVzKGFkanVzdGVkX21vZGUpOwo+IAo+ICAJLyogU3dpdGNoIHRvIGNtZCBtb2RlIGZv ciBwYW5lbC1icmlkZ2UgcHJlX2VuYWJsZSAmIHBhbmVsIHByZXBhcmUgKi8KPiAgCWR3X21pcGlf ZHNpX3NldF9tb2RlKGRzaSwgMCk7CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBt YWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3Rz LmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Mon, 29 Jan 2018 11:46:35 +0200 Subject: [PATCH] drm/bridge/synopsys: dsi: use adjusted_mode in mode_set In-Reply-To: <20180125155504.8611-1-philippe.cornu@st.com> References: <20180125155504.8611-1-philippe.cornu@st.com> Message-ID: <2782155.lsFkNecG25@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Philippe, (CC'ing Daniel Vetter) Thank you for the patch. On Thursday, 25 January 2018 17:55:04 EET Philippe Cornu wrote: > The "adjusted_mode" clock value (ie the real pixel clock) is more > accurate than "mode" clock value (ie the panel/bridge requested > clock value). 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). > > Signed-off-by: Philippe Cornu The adjusted mode is documented as /** * @adjusted_mode: * * Internal display timings which can be used by the driver to handle * differences between the mode requested by userspace in @mode and what * is actually programmed into the hardware. It is purely driver * implementation defined what exactly this adjusted mode means. Usually * it is used to store the hardware display timings used between the * CRTC and encoder blocks. */ This is easy to handle when the CRTC and encoder are controlled by the same driver, as the field is "implementation defined" by a single driver . However, when using bridges, there are two drivers involved, and they must both agree to meaningfully use the adjusted mode. I can't see how to do so without standardizing the meaning of the adjusted mode field. Daniel, what's your opinion on this ? > --- > Note: This patch replaces "drm/bridge/synopsys: dsi: add optional pixel > clock" > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 12 ++++++------ > 1 file changed, 6 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..b926b62e9e33 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -707,20 +707,20 @@ static void dw_mipi_dsi_bridge_mode_set(struct > drm_bridge *bridge, > > clk_prepare_enable(dsi->pclk); > > - ret = phy_ops->get_lane_mbps(priv_data, mode, dsi->mode_flags, > + ret = phy_ops->get_lane_mbps(priv_data, adjusted_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, adjusted_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, adjusted_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, adjusted_mode); > + dw_mipi_dsi_vertical_timing_config(dsi, adjusted_mode); > > dw_mipi_dsi_dphy_init(dsi); > dw_mipi_dsi_dphy_timing_config(dsi); > @@ -734,7 +734,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(adjusted_mode); > > /* Switch to cmd mode for panel-bridge pre_enable & panel prepare */ > dw_mipi_dsi_set_mode(dsi, 0); -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751363AbeA2JqY (ORCPT ); Mon, 29 Jan 2018 04:46:24 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:53413 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750959AbeA2JqW (ORCPT ); Mon, 29 Jan 2018 04:46:22 -0500 From: Laurent Pinchart To: Philippe Cornu Cc: Archit Taneja , Andrzej Hajda , David Airlie , Brian Norris , 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 , daniel.vetter@ffwll.ch Subject: Re: [PATCH] drm/bridge/synopsys: dsi: use adjusted_mode in mode_set Date: Mon, 29 Jan 2018 11:46:35 +0200 Message-ID: <2782155.lsFkNecG25@avalon> Organization: Ideas on Board Oy In-Reply-To: <20180125155504.8611-1-philippe.cornu@st.com> References: <20180125155504.8611-1-philippe.cornu@st.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 Philippe, (CC'ing Daniel Vetter) Thank you for the patch. On Thursday, 25 January 2018 17:55:04 EET Philippe Cornu wrote: > The "adjusted_mode" clock value (ie the real pixel clock) is more > accurate than "mode" clock value (ie the panel/bridge requested > clock value). 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). > > Signed-off-by: Philippe Cornu The adjusted mode is documented as /** * @adjusted_mode: * * Internal display timings which can be used by the driver to handle * differences between the mode requested by userspace in @mode and what * is actually programmed into the hardware. It is purely driver * implementation defined what exactly this adjusted mode means. Usually * it is used to store the hardware display timings used between the * CRTC and encoder blocks. */ This is easy to handle when the CRTC and encoder are controlled by the same driver, as the field is "implementation defined" by a single driver . However, when using bridges, there are two drivers involved, and they must both agree to meaningfully use the adjusted mode. I can't see how to do so without standardizing the meaning of the adjusted mode field. Daniel, what's your opinion on this ? > --- > Note: This patch replaces "drm/bridge/synopsys: dsi: add optional pixel > clock" > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 12 ++++++------ > 1 file changed, 6 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..b926b62e9e33 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -707,20 +707,20 @@ static void dw_mipi_dsi_bridge_mode_set(struct > drm_bridge *bridge, > > clk_prepare_enable(dsi->pclk); > > - ret = phy_ops->get_lane_mbps(priv_data, mode, dsi->mode_flags, > + ret = phy_ops->get_lane_mbps(priv_data, adjusted_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, adjusted_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, adjusted_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, adjusted_mode); > + dw_mipi_dsi_vertical_timing_config(dsi, adjusted_mode); > > dw_mipi_dsi_dphy_init(dsi); > dw_mipi_dsi_dphy_timing_config(dsi); > @@ -734,7 +734,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(adjusted_mode); > > /* Switch to cmd mode for panel-bridge pre_enable & panel prepare */ > dw_mipi_dsi_set_mode(dsi, 0); -- Regards, Laurent Pinchart