From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:47100 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726099AbeHTXTZ (ORCPT ); Mon, 20 Aug 2018 19:19:25 -0400 From: Laurent Pinchart To: Jacopo Mondi Cc: David Airlie , ulrich.hecht+renesas@gmail.com, kieran.bingham@ideasonboard.com, "open list:DRM DRIVERS FOR RENESAS" , "open list:DRM DRIVERS FOR RENESAS" , Jacopo Mondi Subject: Re: [PATCH v2 2/2] drm: rcar-du: Improve non-DPLL clock selection Date: Mon, 20 Aug 2018 23:03:21 +0300 Message-ID: <2548120.ZfZBKIsRRX@avalon> In-Reply-To: <1534778777-6002-3-git-send-email-jacopo+renesas@jmondi.org> References: <1534778777-6002-1-git-send-email-jacopo+renesas@jmondi.org> <1534778777-6002-3-git-send-email-jacopo+renesas@jmondi.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Jacopo, Thank you for the patch. On Monday, 20 August 2018 18:26:17 EEST Jacopo Mondi wrote: > From: Jacopo Mondi > > DU channels not equipped with a DPLL use an internal (aka SoC provided) or I'd say "SoC internal (provided by the CPG)" > external clock source combined with an internal divider to generate the and here "a DU internal divider" to avoid confusion with an SoC internal divider outside of the DU. > desired output dot clock frequency. > > The current clock selection procedure does not fully exploit the ability > of external clock sources to generate the exact dot clock frequency by > themselves, but relies instead on tuning the internal DU clock divider > only, resulting in a less precise clock generation process. > > When possible, and desirable, ask the external clock source for the exact > output dot clock frequency, and test the returned frequency against the > internally generated one to better approximate the desired output dot clock. To make this a big more generic, I' say "and select the clock source that produces the frequency closest to the desired output dot clock". > This patch specifically targets platforms (like Salvator-X[S] and ULCBs) > where the DU's input dotclock.in is generated by the versaclock VC5 clock > source, which is capable of generating the exact rate the DU needs as pixel > clock output. > > This patch fixes higher resolution modes wich requires an high pixel clock s/wich/which/ > output currently not working on non-HDMI DU channel (as VGA 1920x1080@60Hz). Maybe "(such as 1920x1080@60Hz on the VGA output)" ? > > Fixes: 1b30dbde8 "drm: rcar-du: Add support for external pixel clock" Please use the output of git show --pretty=fixes to generate the fixes line. Your SHA1 needs more digits, and the subject should be enclosed with parentheses. > Signed-off-by: Jacopo Mondi > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 53 +++++++++++++++++++------------ > 1 file changed, 32 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 077e681..98ae697 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -258,42 +258,53 @@ static void rcar_du_crtc_set_display_timing(struct > rcar_du_crtc *rcrtc) > > escr = ESCR_DCLKSEL_DCLKIN | div; > } else { > - unsigned long clk; > + unsigned long dotclkin_rate; > + struct clk *dotclkin_clk; > + unsigned long cpg_dist; > u32 div; > > /* > * Compute the clock divisor and select the internal or external > * dot clock based on the requested frequency. > */ > - clk = clk_get_rate(rcrtc->clock); > - div = DIV_ROUND_CLOSEST(clk, mode_clock); > - div = clamp(div, 1U, 64U) - 1; > - > + dotclkin_clk = rcrtc->clock; > + dotclkin_rate = clk_get_rate(rcrtc->clock); > + div = clamp(DIV_ROUND_CLOSEST(dotclkin_rate, mode_clock), > + 1UL, 64UL) - 1; > + cpg_dist = abs(dotclkin_rate / (div + 1) - mode_clock); > escr = ESCR_DCLKSEL_CLKS | div; > > if (rcrtc->extclock) { > - unsigned long extclk; > - unsigned long extrate; > - unsigned long rate; > - u32 extdiv; > - > - extclk = clk_get_rate(rcrtc->extclock); > - extdiv = DIV_ROUND_CLOSEST(extclk, mode_clock); > - extdiv = clamp(extdiv, 1U, 64U) - 1; > + unsigned long ext_rate; > + unsigned long ext_dist; > > - extrate = extclk / (extdiv + 1); > - rate = clk / (div + 1); > + /* > + * If an external clock source is present ask it > + * for the exact dot clock rate, and test the > + * returned value against the cpg provided one. > + */ > + ext_rate = clk_round_rate(rcrtc->extclock, > + mode_clock); > > - if (abs((long)extrate - (long)mode_clock) < > - abs((long)rate - (long)mode_clock)) > - escr = ESCR_DCLKSEL_DCLKIN | extdiv; > + div = clamp(DIV_ROUND_CLOSEST(ext_rate, mode_clock), > + 1UL, 64UL) - 1; > + ext_dist = abs(ext_rate / (div + 1) - mode_clock); > > - dev_dbg(rcrtc->group->dev->dev, > - "mode clock %lu extrate %lu rate %lu ESCR 0x%08x\n", > - mode_clock, extrate, rate, escr); > + if (ext_dist < cpg_dist) { > + dotclkin_clk = rcrtc->extclock; > + dotclkin_rate = ext_rate; > + escr = ESCR_DCLKSEL_DCLKIN | div; > + } > } I think we can do something much simpler here by factoring some code out to a function. I'll send a proposal in reply to this e-mail. > + dev_dbg(rcrtc->group->dev->dev, "mode clock %lu %s rate %lu\n", > + mode_clock, dotclkin_clk == rcrtc->clock ? "cpg" : "ext", > + dotclkin_rate); > + clk_set_rate(dotclkin_clk, dotclkin_rate); > } > > + dev_dbg(rcrtc->group->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr); > + > rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? ESCR2 : ESCR, > escr); > rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? OTAR2 : OTAR, 0); -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 2/2] drm: rcar-du: Improve non-DPLL clock selection Date: Mon, 20 Aug 2018 23:03:21 +0300 Message-ID: <2548120.ZfZBKIsRRX@avalon> References: <1534778777-6002-1-git-send-email-jacopo+renesas@jmondi.org> <1534778777-6002-3-git-send-email-jacopo+renesas@jmondi.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id F343B6E11B for ; Mon, 20 Aug 2018 20:02:42 +0000 (UTC) In-Reply-To: <1534778777-6002-3-git-send-email-jacopo+renesas@jmondi.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jacopo Mondi Cc: Jacopo Mondi , David Airlie , kieran.bingham@ideasonboard.com, "open list:DRM DRIVERS FOR RENESAS" , "open list:DRM DRIVERS FOR RENESAS" , ulrich.hecht+renesas@gmail.com List-Id: dri-devel@lists.freedesktop.org SGkgSmFjb3BvLAoKVGhhbmsgeW91IGZvciB0aGUgcGF0Y2guCgpPbiBNb25kYXksIDIwIEF1Z3Vz dCAyMDE4IDE4OjI2OjE3IEVFU1QgSmFjb3BvIE1vbmRpIHdyb3RlOgo+IEZyb206IEphY29wbyBN b25kaSA8amFjb3BvQGptb25kaS5vcmc+Cj4gCj4gRFUgY2hhbm5lbHMgbm90IGVxdWlwcGVkIHdp dGggYSBEUExMIHVzZSBhbiBpbnRlcm5hbCAoYWthIFNvQyBwcm92aWRlZCkgb3IKCkknZCBzYXkg IlNvQyBpbnRlcm5hbCAocHJvdmlkZWQgYnkgdGhlIENQRykiCgo+IGV4dGVybmFsIGNsb2NrIHNv dXJjZSBjb21iaW5lZCB3aXRoIGFuIGludGVybmFsIGRpdmlkZXIgdG8gZ2VuZXJhdGUgdGhlCgph bmQgaGVyZSAiYSBEVSBpbnRlcm5hbCBkaXZpZGVyIiB0byBhdm9pZCBjb25mdXNpb24gd2l0aCBh biBTb0MgaW50ZXJuYWwgCmRpdmlkZXIgb3V0c2lkZSBvZiB0aGUgRFUuCgo+IGRlc2lyZWQgb3V0 cHV0IGRvdCBjbG9jayBmcmVxdWVuY3kuCj4gCj4gVGhlIGN1cnJlbnQgY2xvY2sgc2VsZWN0aW9u IHByb2NlZHVyZSBkb2VzIG5vdCBmdWxseSBleHBsb2l0IHRoZSBhYmlsaXR5Cj4gb2YgZXh0ZXJu YWwgY2xvY2sgc291cmNlcyB0byBnZW5lcmF0ZSB0aGUgZXhhY3QgZG90IGNsb2NrIGZyZXF1ZW5j eSBieQo+IHRoZW1zZWx2ZXMsIGJ1dCByZWxpZXMgaW5zdGVhZCBvbiB0dW5pbmcgdGhlIGludGVy bmFsIERVIGNsb2NrIGRpdmlkZXIKPiBvbmx5LCByZXN1bHRpbmcgaW4gYSBsZXNzIHByZWNpc2Ug Y2xvY2sgZ2VuZXJhdGlvbiBwcm9jZXNzLgo+IAo+IFdoZW4gcG9zc2libGUsIGFuZCBkZXNpcmFi bGUsIGFzayB0aGUgZXh0ZXJuYWwgY2xvY2sgc291cmNlIGZvciB0aGUgZXhhY3QKPiBvdXRwdXQg ZG90IGNsb2NrIGZyZXF1ZW5jeSwgYW5kIHRlc3QgdGhlIHJldHVybmVkIGZyZXF1ZW5jeSBhZ2Fp bnN0IHRoZQo+IGludGVybmFsbHkgZ2VuZXJhdGVkIG9uZSB0byBiZXR0ZXIgYXBwcm94aW1hdGUg dGhlIGRlc2lyZWQgb3V0cHV0IGRvdCBjbG9jay4KClRvIG1ha2UgdGhpcyBhIGJpZyBtb3JlIGdl bmVyaWMsIEknIHNheSAiYW5kIHNlbGVjdCB0aGUgY2xvY2sgc291cmNlIHRoYXQgCnByb2R1Y2Vz IHRoZSBmcmVxdWVuY3kgY2xvc2VzdCB0byB0aGUgZGVzaXJlZCBvdXRwdXQgZG90IGNsb2NrIi4K Cj4gVGhpcyBwYXRjaCBzcGVjaWZpY2FsbHkgdGFyZ2V0cyBwbGF0Zm9ybXMgKGxpa2UgU2FsdmF0 b3ItWFtTXSBhbmQgVUxDQnMpCj4gd2hlcmUgdGhlIERVJ3MgaW5wdXQgZG90Y2xvY2suaW4gaXMg Z2VuZXJhdGVkIGJ5IHRoZSB2ZXJzYWNsb2NrIFZDNSBjbG9jawo+IHNvdXJjZSwgd2hpY2ggaXMg Y2FwYWJsZSBvZiBnZW5lcmF0aW5nIHRoZSBleGFjdCByYXRlIHRoZSBEVSBuZWVkcyBhcyBwaXhl bAo+IGNsb2NrIG91dHB1dC4KPiAKPiBUaGlzIHBhdGNoIGZpeGVzIGhpZ2hlciByZXNvbHV0aW9u IG1vZGVzIHdpY2ggcmVxdWlyZXMgYW4gaGlnaCBwaXhlbCBjbG9jawoKcy93aWNoL3doaWNoLwoK PiBvdXRwdXQgY3VycmVudGx5IG5vdCB3b3JraW5nIG9uIG5vbi1IRE1JIERVIGNoYW5uZWwgKGFz IFZHQSAxOTIweDEwODBANjBIeikuCgpNYXliZSAiKHN1Y2ggYXMgMTkyMHgxMDgwQDYwSHogb24g dGhlIFZHQSBvdXRwdXQpIiA/Cj4gCj4gRml4ZXM6IDFiMzBkYmRlOCAiZHJtOiByY2FyLWR1OiBB ZGQgc3VwcG9ydCBmb3IgZXh0ZXJuYWwgcGl4ZWwgY2xvY2siCgpQbGVhc2UgdXNlIHRoZSBvdXRw dXQgb2YgZ2l0IHNob3cgLS1wcmV0dHk9Zml4ZXMgdG8gZ2VuZXJhdGUgdGhlIGZpeGVzIGxpbmUu IApZb3VyIFNIQTEgbmVlZHMgbW9yZSBkaWdpdHMsIGFuZCB0aGUgc3ViamVjdCBzaG91bGQgYmUg ZW5jbG9zZWQgd2l0aCAKcGFyZW50aGVzZXMuCgo+IFNpZ25lZC1vZmYtYnk6IEphY29wbyBNb25k aSA8amFjb3BvQGptb25kaS5vcmc+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3Jj YXJfZHVfY3J0Yy5jIHwgNTMgKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLQo+ICAxIGZp bGUgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwgMjEgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfY3J0Yy5jCj4gYi9kcml2ZXJz L2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2NydGMuYyBpbmRleCAwNzdlNjgxLi45OGFlNjk3IDEw MDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfY3J0Yy5jCj4gKysr IGIvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9jcnRjLmMKPiBAQCAtMjU4LDQyICsy NTgsNTMgQEAgc3RhdGljIHZvaWQgcmNhcl9kdV9jcnRjX3NldF9kaXNwbGF5X3RpbWluZyhzdHJ1 Y3QKPiByY2FyX2R1X2NydGMgKnJjcnRjKQo+IAo+ICAJCWVzY3IgPSBFU0NSX0RDTEtTRUxfRENM S0lOIHwgZGl2Owo+ICAJfSBlbHNlIHsKPiAtCQl1bnNpZ25lZCBsb25nIGNsazsKPiArCQl1bnNp Z25lZCBsb25nIGRvdGNsa2luX3JhdGU7Cj4gKwkJc3RydWN0IGNsayAqZG90Y2xraW5fY2xrOwo+ ICsJCXVuc2lnbmVkIGxvbmcgY3BnX2Rpc3Q7Cj4gIAkJdTMyIGRpdjsKPiAKPiAgCQkvKgo+ICAJ CSAqIENvbXB1dGUgdGhlIGNsb2NrIGRpdmlzb3IgYW5kIHNlbGVjdCB0aGUgaW50ZXJuYWwgb3Ig ZXh0ZXJuYWwKPiAgCQkgKiBkb3QgY2xvY2sgYmFzZWQgb24gdGhlIHJlcXVlc3RlZCBmcmVxdWVu Y3kuCj4gIAkJICovCj4gLQkJY2xrID0gY2xrX2dldF9yYXRlKHJjcnRjLT5jbG9jayk7Cj4gLQkJ ZGl2ID0gRElWX1JPVU5EX0NMT1NFU1QoY2xrLCBtb2RlX2Nsb2NrKTsKPiAtCQlkaXYgPSBjbGFt cChkaXYsIDFVLCA2NFUpIC0gMTsKPiAtCj4gKwkJZG90Y2xraW5fY2xrID0gcmNydGMtPmNsb2Nr Owo+ICsJCWRvdGNsa2luX3JhdGUgPSBjbGtfZ2V0X3JhdGUocmNydGMtPmNsb2NrKTsKPiArCQlk aXYgPSBjbGFtcChESVZfUk9VTkRfQ0xPU0VTVChkb3RjbGtpbl9yYXRlLCBtb2RlX2Nsb2NrKSwK PiArCQkJICAgIDFVTCwgNjRVTCkgLSAxOwo+ICsJCWNwZ19kaXN0ID0gYWJzKGRvdGNsa2luX3Jh dGUgLyAoZGl2ICsgMSkgLSBtb2RlX2Nsb2NrKTsKPiAgCQllc2NyID0gRVNDUl9EQ0xLU0VMX0NM S1MgfCBkaXY7Cj4gCj4gIAkJaWYgKHJjcnRjLT5leHRjbG9jaykgewo+IC0JCQl1bnNpZ25lZCBs b25nIGV4dGNsazsKPiAtCQkJdW5zaWduZWQgbG9uZyBleHRyYXRlOwo+IC0JCQl1bnNpZ25lZCBs b25nIHJhdGU7Cj4gLQkJCXUzMiBleHRkaXY7Cj4gLQo+IC0JCQlleHRjbGsgPSBjbGtfZ2V0X3Jh dGUocmNydGMtPmV4dGNsb2NrKTsKPiAtCQkJZXh0ZGl2ID0gRElWX1JPVU5EX0NMT1NFU1QoZXh0 Y2xrLCBtb2RlX2Nsb2NrKTsKPiAtCQkJZXh0ZGl2ID0gY2xhbXAoZXh0ZGl2LCAxVSwgNjRVKSAt IDE7Cj4gKwkJCXVuc2lnbmVkIGxvbmcgZXh0X3JhdGU7Cj4gKwkJCXVuc2lnbmVkIGxvbmcgZXh0 X2Rpc3Q7Cj4gCj4gLQkJCWV4dHJhdGUgPSBleHRjbGsgLyAoZXh0ZGl2ICsgMSk7Cj4gLQkJCXJh dGUgPSBjbGsgLyAoZGl2ICsgMSk7Cj4gKwkJCS8qCj4gKwkJCSAqIElmIGFuIGV4dGVybmFsIGNs b2NrIHNvdXJjZSBpcyBwcmVzZW50IGFzayBpdAo+ICsJCQkgKiBmb3IgdGhlIGV4YWN0IGRvdCBj bG9jayByYXRlLCBhbmQgdGVzdCB0aGUKPiArCQkJICogcmV0dXJuZWQgdmFsdWUgYWdhaW5zdCB0 aGUgY3BnIHByb3ZpZGVkIG9uZS4KPiArCQkJICovCj4gKwkJCWV4dF9yYXRlID0gY2xrX3JvdW5k X3JhdGUocmNydGMtPmV4dGNsb2NrLAo+ICsJCQkJCQkgIG1vZGVfY2xvY2spOwo+IAo+IC0JCQlp ZiAoYWJzKChsb25nKWV4dHJhdGUgLSAobG9uZyltb2RlX2Nsb2NrKSA8Cj4gLQkJCSAgICBhYnMo KGxvbmcpcmF0ZSAtIChsb25nKW1vZGVfY2xvY2spKQo+IC0JCQkJZXNjciA9IEVTQ1JfRENMS1NF TF9EQ0xLSU4gfCBleHRkaXY7Cj4gKwkJCWRpdiA9IGNsYW1wKERJVl9ST1VORF9DTE9TRVNUKGV4 dF9yYXRlLCBtb2RlX2Nsb2NrKSwKPiArCQkJCSAgICAxVUwsIDY0VUwpIC0gMTsKPiArCQkJZXh0 X2Rpc3QgPSBhYnMoZXh0X3JhdGUgLyAoZGl2ICsgMSkgLSBtb2RlX2Nsb2NrKTsKPiAKPiAtCQkJ ZGV2X2RiZyhyY3J0Yy0+Z3JvdXAtPmRldi0+ZGV2LAo+IC0JCQkJIm1vZGUgY2xvY2sgJWx1IGV4 dHJhdGUgJWx1IHJhdGUgJWx1IEVTQ1IgMHglMDh4XG4iLAo+IC0JCQkJbW9kZV9jbG9jaywgZXh0 cmF0ZSwgcmF0ZSwgZXNjcik7Cj4gKwkJCWlmIChleHRfZGlzdCA8IGNwZ19kaXN0KSB7Cj4gKwkJ CQlkb3RjbGtpbl9jbGsgPSByY3J0Yy0+ZXh0Y2xvY2s7Cj4gKwkJCQlkb3RjbGtpbl9yYXRlID0g ZXh0X3JhdGU7Cj4gKwkJCQllc2NyID0gRVNDUl9EQ0xLU0VMX0RDTEtJTiB8IGRpdjsKPiArCQkJ fQo+ICAJCX0KCkkgdGhpbmsgd2UgY2FuIGRvIHNvbWV0aGluZyBtdWNoIHNpbXBsZXIgaGVyZSBi eSBmYWN0b3Jpbmcgc29tZSBjb2RlIG91dCB0byBhIApmdW5jdGlvbi4gSSdsbCBzZW5kIGEgcHJv cG9zYWwgaW4gcmVwbHkgdG8gdGhpcyBlLW1haWwuCgo+ICsJCWRldl9kYmcocmNydGMtPmdyb3Vw LT5kZXYtPmRldiwJIm1vZGUgY2xvY2sgJWx1ICVzIHJhdGUgJWx1XG4iLAo+ICsJCQltb2RlX2Ns b2NrLCBkb3RjbGtpbl9jbGsgPT0gcmNydGMtPmNsb2NrID8gImNwZyIgOiAiZXh0IiwKPiArCQkJ ZG90Y2xraW5fcmF0ZSk7Cj4gKwkJY2xrX3NldF9yYXRlKGRvdGNsa2luX2NsaywgZG90Y2xraW5f cmF0ZSk7Cj4gIAl9Cj4gCj4gKwlkZXZfZGJnKHJjcnRjLT5ncm91cC0+ZGV2LT5kZXYsICIlczog RVNDUiAweCUwOHhcbiIsIF9fZnVuY19fLCBlc2NyKTsKPiArCj4gIAlyY2FyX2R1X2dyb3VwX3dy aXRlKHJjcnRjLT5ncm91cCwgcmNydGMtPmluZGV4ICUgMiA/IEVTQ1IyIDogRVNDUiwKPiAgCQkJ ICAgIGVzY3IpOwo+ICAJcmNhcl9kdV9ncm91cF93cml0ZShyY3J0Yy0+Z3JvdXAsIHJjcnRjLT5p bmRleCAlIDIgPyBPVEFSMiA6IE9UQVIsIDApOwoKLS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNo YXJ0CgoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRy aS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRw czovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=