From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Norris Subject: Re: [PATCH 1/7] drm/rockchip/dsi: correct Feedback divider setting Date: Tue, 19 Sep 2017 11:19:01 -0700 Message-ID: <20170919181751.GA38656@google.com> References: <1505725539-6309-1-git-send-email-nickey.yang@rock-chips.com> <20170919180025.apb4aq7ca3filh6c@art_vandelay> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20170919180025.apb4aq7ca3filh6c@art_vandelay> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sean Paul Cc: mark.rutland@arm.com, bivvy.bi@rock-chips.com, hl@rock-chips.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, Nickey Yang , robh+dt@kernel.org, zyw@rock-chips.com, xbl@rock-chips.com List-Id: linux-rockchip.vger.kernel.org SGkgU2VhbiwKCk9uIFR1ZSwgU2VwIDE5LCAyMDE3IGF0IDExOjAwOjI1QU0gLTA3MDAsIFNlYW4g UGF1bCB3cm90ZToKPiBPbiBNb24sIFNlcCAxOCwgMjAxNyBhdCAwNTowNTozM1BNICswODAwLCBO aWNrZXkgWWFuZyB3cm90ZToKPiA+IFRoaXMgcGF0Y2ggY29ycmVjdCBGZWVkYmFjayBkaXZpZGVy IHNldHRpbmc6Cj4gPiAx44CBU2V0IEZlZWRiYWNrIGRpdmlkZXIgWzg6NV0gd2hlbiBISUdIX1BS T0dSQU1fRU4KPiA+IDLjgIFEdWUgdG8gdGhlIHVzZSBvZiBhICJieSAyIHByZS1zY2FsZXIsIiB0 aGUgcmFuZ2Ugb2YgdGhlCj4gPiBmZWVkYmFjayBtdWx0aXBsaWNhdGlvbiBGZWVkYmFjayBkaXZp ZGVyIGlzIGxpbWl0ZWQgdG8gZXZlbgo+ID4gZGl2aXNpb24gbnVtYmVycywgYW5kIEZlZWRiYWNr IGRpdmlkZXIgbXVzdCBiZSBncmVhdGVyIHRoYW4KPiA+IDEyLCBsZXNzIHRoYW4gMTAwMC4KPiA+ IDPjgIFNYWtlIHRoZSBwcmV2aW91c2x5IGNvbmZpZ3VyZWQgRmVlZGJhY2sgZGl2aWRlcihMU0Ip Cj4gPiBmYWN0b3JzIGVmZmVjdGl2ZQo+ID4gCj4gPiBTaWduZWQtb2ZmLWJ5OiBOaWNrZXkgWWFu ZyA8bmlja2V5LnlhbmdAcm9jay1jaGlwcy5jb20+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2dwdS9k cm0vcm9ja2NoaXAvZHctbWlwaS1kc2kuYyB8IDgzICsrKysrKysrKysrKysrKysrKysrKystLS0t LS0tLS0tLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgNTQgaW5zZXJ0aW9ucygrKSwgMjkgZGVsZXRp b25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvZHct bWlwaS1kc2kuYyBiL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaS5jCj4gPiBp bmRleCA5YTIwYjlkLi41MjY5OGI3IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3Jv Y2tjaGlwL2R3LW1pcGktZHNpLmMKPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9k dy1taXBpLWRzaS5jCj4gPiBAQCAtMjI4LDcgKzIyOCw3IEBACj4gPiAgI2RlZmluZSBMT1dfUFJP R1JBTV9FTgkJMAo+ID4gICNkZWZpbmUgSElHSF9QUk9HUkFNX0VOCQlCSVQoNykKPiA+ICAjZGVm aW5lIExPT1BfRElWX0xPV19TRUwodmFsKQkoKCh2YWwpIC0gMSkgJiAweDFmKQo+ID4gLSNkZWZp bmUgTE9PUF9ESVZfSElHSF9TRUwodmFsKQkoKCgodmFsKSAtIDEpID4+IDUpICYgMHgxZikKPiA+ ICsjZGVmaW5lIExPT1BfRElWX0hJR0hfU0VMKHZhbCkJKCgoKHZhbCkgLSAxKSA+PiA1KSAmIDB4 ZikKPiA+ICAjZGVmaW5lIFBMTF9MT09QX0RJVl9FTgkJQklUKDUpCj4gPiAgI2RlZmluZSBQTExf SU5QVVRfRElWX0VOCUJJVCg0KQo+ID4gIAo+ID4gQEAgLTQ2MSw2ICs0NjEsNyBAQCBzdGF0aWMg aW50IGR3X21pcGlfZHNpX3BoeV9pbml0KHN0cnVjdCBkd19taXBpX2RzaSAqZHNpKQo+ID4gIAlk d19taXBpX2RzaV9waHlfd3JpdGUoZHNpLCAweDE3LCBJTlBVVF9ESVZJREVSKGRzaS0+aW5wdXRf ZGl2KSk7Cj4gPiAgCWR3X21pcGlfZHNpX3BoeV93cml0ZShkc2ksIDB4MTgsIExPT1BfRElWX0xP V19TRUwoZHNpLT5mZWVkYmFja19kaXYpIHwKPiA+ICAJCQkJCSBMT1dfUFJPR1JBTV9FTik7Cj4g PiArCWR3X21pcGlfZHNpX3BoeV93cml0ZShkc2ksIDB4MTksIFBMTF9MT09QX0RJVl9FTiB8IFBM TF9JTlBVVF9ESVZfRU4pOwo+IAo+IFlvdSBkbyB0aGUgc2FtZSB3cml0ZSAyIGxpbmVzIGRvd24u IEFyZSBib3RoIG5lZWRlZD8gSXQgd291bGQgYmUgbmljZSBpZiB0aGUKPiByZWdpc3RlciBuYW1l cyB3ZXJlIGFsc28gZGVmaW5lZCwgc28gdGhpcyBpcyBlYXNpZXIgdG8gcmVhZC4KCklmIEknbSBy ZWFkaW5nIGNvcnJlY3RseSwgSSB0aGluayB0aGlzIGlzIHdoYXQgTmlja2V5IG1lYW50IGJ5OgoK IjPjgIFNYWtlIHRoZSBwcmV2aW91c2x5IGNvbmZpZ3VyZWQgRmVlZGJhY2sgZGl2aWRlcihMU0Ip CmZhY3RvcnMgZWZmZWN0aXZlIgoKLiBNeSByZWFkaW5nIG9mIHRoZSBkYXRhYm9vayBpcyB0aGF0 IHRoaXMgc3RlcCBmaW5hbGl6ZXMgdGhlIHByZXZpb3VzCnR3byB3cml0ZXMgKHRvIHRlc3QgY29k ZSAweDE3IGFuZCAweDE4KS4KCkdpdmVuIHRoaXMgd2FzIGJ1Z2d5ICg/KSBwcmV2aW91c2x5LCBp dCBkb2VzIHNlZW0gbGlrZSBoYXZpbmcgc29tZSBleHRyYQpsYW5ndWFnZSB0byBkb2N1bWVudCB0 aGlzIGNvdWxkIGhlbHAuIFJlZ2lzdGVyIG5hbWVzIChvciAidGVzdCBjb2RlcyIsCnBlciB0aGUg ZG9jcz8pIGNvdWxkIGhlbHAsIGJ1dCBhZGRpdGlvbmFsbHksIG1heWJlIGEgZmV3IG1vcmUgY29t bWVudHMuCgo+ID4gIAlkd19taXBpX2RzaV9waHlfd3JpdGUoZHNpLCAweDE4LCBMT09QX0RJVl9I SUdIX1NFTChkc2ktPmZlZWRiYWNrX2RpdikgfAo+ID4gIAkJCQkJIEhJR0hfUFJPR1JBTV9FTik7 Cj4gPiAgCWR3X21pcGlfZHNpX3BoeV93cml0ZShkc2ksIDB4MTksIFBMTF9MT09QX0RJVl9FTiB8 IFBMTF9JTlBVVF9ESVZfRU4pOwoKWy4uLl0KCkJyaWFuCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWls bWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751513AbdISSTI (ORCPT ); Tue, 19 Sep 2017 14:19:08 -0400 Received: from mail-pf0-f182.google.com ([209.85.192.182]:49653 "EHLO mail-pf0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750919AbdISSTG (ORCPT ); Tue, 19 Sep 2017 14:19:06 -0400 X-Google-Smtp-Source: AOwi7QCh9j2HX1dA9krhw/PvWr/iVUYyjsx8NJiACOX7NOQg9dD9DCXoqLFLOtOLFna52wXc6oWaeg== Date: Tue, 19 Sep 2017 11:19:01 -0700 From: Brian Norris To: Sean Paul Cc: Nickey Yang , mark.yao@rock-chips.com, robh+dt@kernel.org, heiko@sntech.de, mark.rutland@arm.com, airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, hl@rock-chips.com, zyw@rock-chips.com, bivvy.bi@rock-chips.com, xbl@rock-chips.com Subject: Re: [PATCH 1/7] drm/rockchip/dsi: correct Feedback divider setting Message-ID: <20170919181751.GA38656@google.com> References: <1505725539-6309-1-git-send-email-nickey.yang@rock-chips.com> <20170919180025.apb4aq7ca3filh6c@art_vandelay> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170919180025.apb4aq7ca3filh6c@art_vandelay> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sean, On Tue, Sep 19, 2017 at 11:00:25AM -0700, Sean Paul wrote: > On Mon, Sep 18, 2017 at 05:05:33PM +0800, Nickey Yang wrote: > > This patch correct Feedback divider setting: > > 1、Set Feedback divider [8:5] when HIGH_PROGRAM_EN > > 2、Due to the use of a "by 2 pre-scaler," the range of the > > feedback multiplication Feedback divider is limited to even > > division numbers, and Feedback divider must be greater than > > 12, less than 1000. > > 3、Make the previously configured Feedback divider(LSB) > > factors effective > > > > Signed-off-by: Nickey Yang > > --- > > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 83 ++++++++++++++++++++++------------ > > 1 file changed, 54 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > index 9a20b9d..52698b7 100644 > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > > @@ -228,7 +228,7 @@ > > #define LOW_PROGRAM_EN 0 > > #define HIGH_PROGRAM_EN BIT(7) > > #define LOOP_DIV_LOW_SEL(val) (((val) - 1) & 0x1f) > > -#define LOOP_DIV_HIGH_SEL(val) ((((val) - 1) >> 5) & 0x1f) > > +#define LOOP_DIV_HIGH_SEL(val) ((((val) - 1) >> 5) & 0xf) > > #define PLL_LOOP_DIV_EN BIT(5) > > #define PLL_INPUT_DIV_EN BIT(4) > > > > @@ -461,6 +461,7 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) > > dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div)); > > dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) | > > LOW_PROGRAM_EN); > > + dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); > > You do the same write 2 lines down. Are both needed? It would be nice if the > register names were also defined, so this is easier to read. If I'm reading correctly, I think this is what Nickey meant by: "3、Make the previously configured Feedback divider(LSB) factors effective" . My reading of the databook is that this step finalizes the previous two writes (to test code 0x17 and 0x18). Given this was buggy (?) previously, it does seem like having some extra language to document this could help. Register names (or "test codes", per the docs?) could help, but additionally, maybe a few more comments. > > dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) | > > HIGH_PROGRAM_EN); > > dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); [...] Brian