From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [01/26] drm/rockchip: dw-mipi-dsi: use mode from display state Date: Wed, 18 Jan 2017 10:21:48 +0800 Message-ID: <587ED13C.3000301@rock-chips.com> References: <20160919171747.28512-2-john@metanate.com> <587DF431.5040003@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <587DF431.5040003@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Zhong , John Keeping Cc: linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: linux-rockchip.vger.kernel.org T24gMjAxN+W5tDAx5pyIMTfml6UgMTg6MzgsIENocmlzIFpob25nIHdyb3RlOgo+PiBAQCAtODIx LDggKzgyNCw2IEBAIHN0YXRpYyB2b2lkIGR3X21pcGlfZHNpX2VuY29kZXJfbW9kZV9zZXQoc3Ry dWN0IAo+PiBkcm1fZW5jb2RlciAqZW5jb2RlciwKPj4gICAgICAgc3RydWN0IGR3X21pcGlfZHNp ICpkc2kgPSBlbmNvZGVyX3RvX2RzaShlbmNvZGVyKTsKPj4gICAgICAgaW50IHJldDsKPj4gICAt ICAgIGRzaS0+bW9kZSA9IGFkanVzdGVkX21vZGU7Cj4+IC0KPiBJIHByZWZlciB0byBrZWVwIHRo ZSBvcmlnaW5hbCBtZXRob2QsIGFsdGhvdWdoIHRoaXMiZHNpLT5tb2RlIiBwb2ludGVyIAo+IGlz IHNhbWUgYXMgIiZkc2ktPmNvbm5lY3Rvci5zdGF0ZS0+Y3J0Yy0+c3RhdGUtPmFkanVzdGVkX21v ZGU7ICIKPiBJIHN0aWxsIHRoaW5rIGRzaS0+bW9kZSBtYWtlcyB0aGUgcHJvY2VzcyBlYXNpZXIg dG8gcmVhZAoiJmRzaS0+Y29ubmVjdG9yLnN0YXRlLT5jcnRjLT5zdGF0ZS0+YWRqdXN0ZWRfbW9k ZTsiIGlzIHRvbyBsb25nLCBhbmQgCmRzaS0+Y29ubmVjdG9yLnN0YXRlLT5jcnRjIGNhbiBiZSBO VUxMLCBzZWVtcyBub3QgZ29vZC4KCmFncmVlIHdpdGggQ2hyaXMsIEkgdGhpbmsgZHNpLT5tb2Rl IGlzIGJldHRlci4KCkJ1ZyB0aGUgb3JpZ2luIGNvZGUgImRzaS0+bW9kZSA9IGFkanVzdGVkX21v ZGU7ICIgYWxzbyBoYXMgYSBidWcsIAphZGp1c3RlZF9tb2RlJ3MgbGlmdCB0aW1lIGlzIHVua25v d24sIHVzZSBhIHBvaW50ZXIgZnJvbSBhZGp1c3RlZF9tb2RlIAppcyBkYW5nZXJvdXMuCgpJIHRo aW5rIHdlIGNhbiB1c2UgbGlrZSB0aGF0LCBjb3B5IGEgZGlzcGxheSBtb2RlIHRvIGRzaS0+bW9k ZToKc3RydWN0IGR3X21pcGlfZHNpCnsKICAgIC0gICAgc3RydWN0IGRybV9kaXNwbGF5X21vZGUg Km1vZGU7CiAgICArICAgIHN0cnVjdCBkcm1fZGlzcGxheV9tb2RlIG1vZGU7Cn0KICAgeHh4X2Vu Y29kZXJfbW9kZV9zZXQoKQp7CiAgICBkcm1fbW9kZV9jb3B5KCZkc2ktPm1vZGUsIGFkanVzdGVk X21vZGUpOwp9CgotLSAK77ytYXJrIFlhbwoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3Rz LmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Wed, 18 Jan 2017 10:21:48 +0800 Subject: [01/26] drm/rockchip: dw-mipi-dsi: use mode from display state In-Reply-To: <587DF431.5040003@rock-chips.com> References: <20160919171747.28512-2-john@metanate.com> <587DF431.5040003@rock-chips.com> Message-ID: <587ED13C.3000301@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2017?01?17? 18:38, Chris Zhong wrote: >> @@ -821,8 +824,6 @@ static void dw_mipi_dsi_encoder_mode_set(struct >> drm_encoder *encoder, >> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); >> int ret; >> - dsi->mode = adjusted_mode; >> - > I prefer to keep the original method, although this"dsi->mode" pointer > is same as "&dsi->connector.state->crtc->state->adjusted_mode; " > I still think dsi->mode makes the process easier to read "&dsi->connector.state->crtc->state->adjusted_mode;" is too long, and dsi->connector.state->crtc can be NULL, seems not good. agree with Chris, I think dsi->mode is better. Bug the origin code "dsi->mode = adjusted_mode; " also has a bug, adjusted_mode's lift time is unknown, use a pointer from adjusted_mode is dangerous. I think we can use like that, copy a display mode to dsi->mode: struct dw_mipi_dsi { - struct drm_display_mode *mode; + struct drm_display_mode mode; } xxx_encoder_mode_set() { drm_mode_copy(&dsi->mode, adjusted_mode); } -- ?ark Yao From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751981AbdARCWh (ORCPT ); Tue, 17 Jan 2017 21:22:37 -0500 Received: from regular1.263xmail.com ([211.150.99.133]:35364 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751183AbdARCWg (ORCPT ); Tue, 17 Jan 2017 21:22:36 -0500 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: <9101329eeedc9780688d4dcbfd69ab39> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [01/26] drm/rockchip: dw-mipi-dsi: use mode from display state To: Chris Zhong , John Keeping References: <20160919171747.28512-2-john@metanate.com> <587DF431.5040003@rock-chips.com> Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org From: Mark yao Message-ID: <587ED13C.3000301@rock-chips.com> Date: Wed, 18 Jan 2017 10:21:48 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <587DF431.5040003@rock-chips.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017年01月17日 18:38, Chris Zhong wrote: >> @@ -821,8 +824,6 @@ static void dw_mipi_dsi_encoder_mode_set(struct >> drm_encoder *encoder, >> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); >> int ret; >> - dsi->mode = adjusted_mode; >> - > I prefer to keep the original method, although this"dsi->mode" pointer > is same as "&dsi->connector.state->crtc->state->adjusted_mode; " > I still think dsi->mode makes the process easier to read "&dsi->connector.state->crtc->state->adjusted_mode;" is too long, and dsi->connector.state->crtc can be NULL, seems not good. agree with Chris, I think dsi->mode is better. Bug the origin code "dsi->mode = adjusted_mode; " also has a bug, adjusted_mode's lift time is unknown, use a pointer from adjusted_mode is dangerous. I think we can use like that, copy a display mode to dsi->mode: struct dw_mipi_dsi { - struct drm_display_mode *mode; + struct drm_display_mode mode; } xxx_encoder_mode_set() { drm_mode_copy(&dsi->mode, adjusted_mode); } -- Ｍark Yao