From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Keeping Subject: Re: [PATCH 1/7] drm/rockchip/dsi: correct Feedback divider setting Date: Wed, 20 Sep 2017 11:08:51 +0100 Message-ID: <20170920100851.GI1601@john.keeping.me.uk> References: <1505725539-6309-1-git-send-email-nickey.yang@rock-chips.com> <20170919180025.apb4aq7ca3filh6c@art_vandelay> <20170919181751.GA38656@google.com> <20170919202740.2ku56pefrj3wyskw@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: <20170919202740.2ku56pefrj3wyskw@art_vandelay> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Sean Paul Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org, bivvy.bi-TNX95d0MmH7DzftRWevZcw@public.gmane.org, hl-TNX95d0MmH7DzftRWevZcw@public.gmane.org, airlied-cv59FeDIM0c@public.gmane.org, Brian Norris , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Nickey Yang , robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org, xbl-TNX95d0MmH7DzftRWevZcw@public.gmane.org, mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org, heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org List-Id: linux-rockchip.vger.kernel.org T24gVHVlLCBTZXAgMTksIDIwMTcgYXQgMDE6Mjc6NDBQTSAtMDcwMCwgU2VhbiBQYXVsIHdyb3Rl Ogo+IE9uIFR1ZSwgU2VwIDE5LCAyMDE3IGF0IDExOjE5OjAxQU0gLTA3MDAsIEJyaWFuIE5vcnJp cyB3cm90ZToKPiA+IEhpIFNlYW4sCj4gPiAKPiA+IE9uIFR1ZSwgU2VwIDE5LCAyMDE3IGF0IDEx OjAwOjI1QU0gLTA3MDAsIFNlYW4gUGF1bCB3cm90ZToKPiA+ID4gT24gTW9uLCBTZXAgMTgsIDIw MTcgYXQgMDU6MDU6MzNQTSArMDgwMCwgTmlja2V5IFlhbmcgd3JvdGU6Cj4gPiA+ID4gVGhpcyBw YXRjaCBjb3JyZWN0IEZlZWRiYWNrIGRpdmlkZXIgc2V0dGluZzoKPiA+ID4gPiAx44CBU2V0IEZl ZWRiYWNrIGRpdmlkZXIgWzg6NV0gd2hlbiBISUdIX1BST0dSQU1fRU4KPiA+ID4gPiAy44CBRHVl IHRvIHRoZSB1c2Ugb2YgYSAiYnkgMiBwcmUtc2NhbGVyLCIgdGhlIHJhbmdlIG9mIHRoZQo+ID4g PiA+IGZlZWRiYWNrIG11bHRpcGxpY2F0aW9uIEZlZWRiYWNrIGRpdmlkZXIgaXMgbGltaXRlZCB0 byBldmVuCj4gPiA+ID4gZGl2aXNpb24gbnVtYmVycywgYW5kIEZlZWRiYWNrIGRpdmlkZXIgbXVz dCBiZSBncmVhdGVyIHRoYW4KPiA+ID4gPiAxMiwgbGVzcyB0aGFuIDEwMDAuCj4gPiA+ID4gM+OA gU1ha2UgdGhlIHByZXZpb3VzbHkgY29uZmlndXJlZCBGZWVkYmFjayBkaXZpZGVyKExTQikKPiA+ ID4gPiBmYWN0b3JzIGVmZmVjdGl2ZQo+ID4gPiA+IAo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IE5p Y2tleSBZYW5nIDxuaWNrZXkueWFuZ0Byb2NrLWNoaXBzLmNvbT4KPiA+ID4gPiAtLS0KPiA+ID4g PiAgZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMgfCA4MyArKysrKysrKysr KysrKysrKysrKysrLS0tLS0tLS0tLS0tCj4gPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCA1NCBpbnNl cnRpb25zKCspLCAyOSBkZWxldGlvbnMoLSkKPiA+ID4gPiAKPiA+ID4gPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMgYi9kcml2ZXJzL2dwdS9kcm0v cm9ja2NoaXAvZHctbWlwaS1kc2kuYwo+ID4gPiA+IGluZGV4IDlhMjBiOWQuLjUyNjk4YjcgMTAw NjQ0Cj4gPiA+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNpLmMK PiA+ID4gPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvZHctbWlwaS1kc2kuYwo+ID4g PiA+IEBAIC0yMjgsNyArMjI4LDcgQEAKPiA+ID4gPiAgI2RlZmluZSBMT1dfUFJPR1JBTV9FTgkJ MAo+ID4gPiA+ICAjZGVmaW5lIEhJR0hfUFJPR1JBTV9FTgkJQklUKDcpCj4gPiA+ID4gICNkZWZp bmUgTE9PUF9ESVZfTE9XX1NFTCh2YWwpCSgoKHZhbCkgLSAxKSAmIDB4MWYpCj4gPiA+ID4gLSNk ZWZpbmUgTE9PUF9ESVZfSElHSF9TRUwodmFsKQkoKCgodmFsKSAtIDEpID4+IDUpICYgMHgxZikK PiA+ID4gPiArI2RlZmluZSBMT09QX0RJVl9ISUdIX1NFTCh2YWwpCSgoKCh2YWwpIC0gMSkgPj4g NSkgJiAweGYpCj4gPiA+ID4gICNkZWZpbmUgUExMX0xPT1BfRElWX0VOCQlCSVQoNSkKPiA+ID4g PiAgI2RlZmluZSBQTExfSU5QVVRfRElWX0VOCUJJVCg0KQo+ID4gPiA+ICAKPiA+ID4gPiBAQCAt NDYxLDYgKzQ2MSw3IEBAIHN0YXRpYyBpbnQgZHdfbWlwaV9kc2lfcGh5X2luaXQoc3RydWN0IGR3 X21pcGlfZHNpICpkc2kpCj4gPiA+ID4gIAlkd19taXBpX2RzaV9waHlfd3JpdGUoZHNpLCAweDE3 LCBJTlBVVF9ESVZJREVSKGRzaS0+aW5wdXRfZGl2KSk7Cj4gPiA+ID4gIAlkd19taXBpX2RzaV9w aHlfd3JpdGUoZHNpLCAweDE4LCBMT09QX0RJVl9MT1dfU0VMKGRzaS0+ZmVlZGJhY2tfZGl2KSB8 Cj4gPiA+ID4gIAkJCQkJIExPV19QUk9HUkFNX0VOKTsKPiA+ID4gPiArCWR3X21pcGlfZHNpX3Bo eV93cml0ZShkc2ksIDB4MTksIFBMTF9MT09QX0RJVl9FTiB8IFBMTF9JTlBVVF9ESVZfRU4pOwo+ ID4gPiAKPiA+ID4gWW91IGRvIHRoZSBzYW1lIHdyaXRlIDIgbGluZXMgZG93bi4gQXJlIGJvdGgg bmVlZGVkPyBJdCB3b3VsZCBiZSBuaWNlIGlmIHRoZQo+ID4gPiByZWdpc3RlciBuYW1lcyB3ZXJl IGFsc28gZGVmaW5lZCwgc28gdGhpcyBpcyBlYXNpZXIgdG8gcmVhZC4KPiA+IAo+ID4gSWYgSSdt IHJlYWRpbmcgY29ycmVjdGx5LCBJIHRoaW5rIHRoaXMgaXMgd2hhdCBOaWNrZXkgbWVhbnQgYnk6 Cj4gPiAKPiA+ICIz44CBTWFrZSB0aGUgcHJldmlvdXNseSBjb25maWd1cmVkIEZlZWRiYWNrIGRp dmlkZXIoTFNCKQo+ID4gZmFjdG9ycyBlZmZlY3RpdmUiCj4gPiAKPiA+IC4gTXkgcmVhZGluZyBv ZiB0aGUgZGF0YWJvb2sgaXMgdGhhdCB0aGlzIHN0ZXAgZmluYWxpemVzIHRoZSBwcmV2aW91cwo+ ID4gdHdvIHdyaXRlcyAodG8gdGVzdCBjb2RlIDB4MTcgYW5kIDB4MTgpLgo+ID4gCj4gPiBHaXZl biB0aGlzIHdhcyBidWdneSAoPykgcHJldmlvdXNseSwgaXQgZG9lcyBzZWVtIGxpa2UgaGF2aW5n IHNvbWUgZXh0cmEKPiA+IGxhbmd1YWdlIHRvIGRvY3VtZW50IHRoaXMgY291bGQgaGVscC4gUmVn aXN0ZXIgbmFtZXMgKG9yICJ0ZXN0IGNvZGVzIiwKPiA+IHBlciB0aGUgZG9jcz8pIGNvdWxkIGhl bHAsIGJ1dCBhZGRpdGlvbmFsbHksIG1heWJlIGEgZmV3IG1vcmUgY29tbWVudHMuCj4gPiAKPiAK PiBBaCwgeWVhaCwgdGhhbmtzIGZvciB0aGUgZXhwbGFuYXRpb24uIEl0J3Mgbm90IGNsZWFyIHRo YXQgdGhpcyBsYXRjaGVzIHRoZSAKPiB2YWx1ZXMgYWJvdmUuIEkgdGhpbmsgcmVnaXN0ZXIgbmFt ZXMgYW5kIGNvbW1lbnRzIHdvdWxkIGJlIGltbWVuc2VseSBoZWxwZnVsLgoKQWNjb3JkaW5nIHRv IHRoZSBkYXRhYm9vaywgMHgxOSBjb250cm9scyB3aGV0aGVyIHRoZSBsb29wL2lucHV0IGRpdmlk ZXJzCmFyZSBkZXJpdmVkIGZyb20gdGhlIGhzZnJlcXJhbmdlIGNvbmZpZ3VyYXRpb24gb3IgdXNl IHRoZSB2YWx1ZXMgaW4gMHgxNwphbmQgMHgxOC4gIEkgY2FuJ3Qgc2VlIHdoeSB3cml0aW5nIHRo ZSBzYW1lIHZhbHVlIHRvIHRoaXMgcmVnaXN0ZXIKbXVsdGlwbGUgdGltZXMgaXMgbmVjZXNzYXJ5 LgoKPiA+ID4gPiAgCWR3X21pcGlfZHNpX3BoeV93cml0ZShkc2ksIDB4MTgsIExPT1BfRElWX0hJ R0hfU0VMKGRzaS0+ZmVlZGJhY2tfZGl2KSB8Cj4gPiA+ID4gIAkJCQkJIEhJR0hfUFJPR1JBTV9F Tik7Cj4gPiA+ID4gIAlkd19taXBpX2RzaV9waHlfd3JpdGUoZHNpLCAweDE5LCBQTExfTE9PUF9E SVZfRU4gfCBQTExfSU5QVVRfRElWX0VOKTsKPiA+IAo+ID4gWy4uLl0KCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4LXJvY2tjaGlwIG1haWxpbmcg bGlzdApMaW51eC1yb2NrY2hpcEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZy YWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtcm9ja2NoaXAK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751800AbdITKRn convert rfc822-to-8bit (ORCPT ); Wed, 20 Sep 2017 06:17:43 -0400 Received: from mta01.prd.rdg.aluminati.org ([94.76.243.214]:48263 "EHLO mta01.prd.rdg.aluminati.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751480AbdITKRm (ORCPT ); Wed, 20 Sep 2017 06:17:42 -0400 X-Greylist: delayed 496 seconds by postgrey-1.27 at vger.kernel.org; Wed, 20 Sep 2017 06:17:41 EDT X-Quarantine-ID: X-Spam-Flag: NO X-Spam-Score: -0.249 Date: Wed, 20 Sep 2017 11:08:51 +0100 From: John Keeping To: Sean Paul Cc: Brian Norris , mark.rutland@arm.com, bivvy.bi@rock-chips.com, hl@rock-chips.com, heiko@sntech.de, airlied@linux.ie, 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, mark.yao@rock-chips.com Subject: Re: [PATCH 1/7] drm/rockchip/dsi: correct Feedback divider setting Message-ID: <20170920100851.GI1601@john.keeping.me.uk> References: <1505725539-6309-1-git-send-email-nickey.yang@rock-chips.com> <20170919180025.apb4aq7ca3filh6c@art_vandelay> <20170919181751.GA38656@google.com> <20170919202740.2ku56pefrj3wyskw@art_vandelay> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170919202740.2ku56pefrj3wyskw@art_vandelay> User-Agent: Mutt/1.8.3 (2017-05-23) Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 19, 2017 at 01:27:40PM -0700, Sean Paul wrote: > On Tue, Sep 19, 2017 at 11:19:01AM -0700, Brian Norris wrote: > > 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. > > > > Ah, yeah, thanks for the explanation. It's not clear that this latches the > values above. I think register names and comments would be immensely helpful. According to the databook, 0x19 controls whether the loop/input dividers are derived from the hsfreqrange configuration or use the values in 0x17 and 0x18. I can't see why writing the same value to this register multiple times is necessary. > > > > 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); > > > > [...]