Message ID | 20170214134004.GA8570@amd (mailing list archive) |
---|---|
State | New |
Delegated to: | Laurent Pinchart |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1cddLA-0001Su-2g; Tue, 14 Feb 2017 13:40:20 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-6) with esmtp id 1cddL8-000704-3W; Tue, 14 Feb 2017 14:40:19 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753990AbdBNNkM (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Tue, 14 Feb 2017 08:40:12 -0500 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:59112 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753979AbdBNNkH (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 14 Feb 2017 08:40:07 -0500 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id 882BA817DB; Tue, 14 Feb 2017 14:40:05 +0100 (CET) Date: Tue, 14 Feb 2017 14:40:04 +0100 From: Pavel Machek <pavel@ucw.cz> To: sakari.ailus@iki.fi Cc: sre@kernel.org, pali.rohar@gmail.com, pavel@ucw.cz, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, mchehab@kernel.org, ivo.g.dimitrov.75@gmail.com Subject: [RFC 08/13] smiapp-pll: Take existing divisor into account in minimum divisor check Message-ID: <20170214134004.GA8570@amd> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mYCpIKhGyMATD0i+" Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2017.2.14.133016 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' BODY_PARA_IS_SENTENCE_URL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, KNOWN_FREEWEB_URI 0.05, MSGID_ADDED_BY_MTA 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, INVALID_MSGID_NO_FQDN 0, LEGITIMATE_SIGNS 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __ATTACHMENT_SIZE_0_10K 0, __CD 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FROM_DOMAIN_IN_ANY_CC2 0, __FROM_DOMAIN_IN_RCPT 0, __HAS_ATTACHMENT 0, __HAS_ATTACHMENT1 0, __HAS_ATTACHMENT2 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_LIST_ID 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __KNOWN_FREEWEB_URI2 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MIME_TEXT_P2 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __NO_HTML_TAG_RAW 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NS , __URI_WITH_PATH 0, __USER_AGENT 0' |
Commit Message
Pavel Machek
Feb. 14, 2017, 1:40 p.m. UTC
From: Sakari Ailus <sakari.ailus@iki.fi> Required added multiplier (and divisor) calculation did not take into account the existing divisor when checking the values against the minimum divisor. Do just that. Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> Signed-off-by: Pavel Machek <pavel@ucw.cz> --- drivers/media/i2c/smiapp-pll.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Comments
Hi Pavel, On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote: > From: Sakari Ailus <sakari.ailus@iki.fi> > > Required added multiplier (and divisor) calculation did not take into > account the existing divisor when checking the values against the > minimum divisor. Do just that. > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > Signed-off-by: Pavel Machek <pavel@ucw.cz> I need to understand again why did I write this patch. :-) Could you send me the smiapp driver output with debug level messages enabled, please? I think the problem was with the secondary sensor.
Hi Sakari, On Wednesday 15 Feb 2017 00:05:03 Sakari Ailus wrote: > On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote: > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > Required added multiplier (and divisor) calculation did not take into > > account the existing divisor when checking the values against the > > minimum divisor. Do just that. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > I need to understand again why did I write this patch. :-) I was about to mention that a more detailed commit message (or possibly event comments in the source code) would be good :-) > Could you send me the smiapp driver output with debug level messages > enabled, please? > > I think the problem was with the secondary sensor. > > > diff --git a/drivers/media/i2c/smiapp-pll.c > > b/drivers/media/i2c/smiapp-pll.c > > index 771db56..166bbaf 100644 > > --- a/drivers/media/i2c/smiapp-pll.c > > +++ b/drivers/media/i2c/smiapp-pll.c > > @@ -16,6 +16,8 @@ > > * General Public License for more details. > > */ > > > > +#define DEBUG > > + This should be removed. > > #include <linux/device.h> > > #include <linux/gcd.h> > > #include <linux/lcm.h> > > @@ -227,7 +229,8 @@ static int __smiapp_pll_calculate( > > more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div; > > dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor); > > > > - more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div); > > + more_mul_factor = lcm(more_mul_factor, > > + DIV_ROUND_UP(op_limits->min_sys_clk_div, div)); > > dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n", > > more_mul_factor); > > i = roundup(more_mul_min, more_mul_factor); > > @@ -456,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev, > > i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz); > > mul = div_u64(pll->pll_op_clk_freq_hz, i); > > div = pll->ext_clk_freq_hz / i; > > + if (!mul) { > > + dev_err(dev, "forcing mul to 1\n"); > > + mul = 1; > > + } > > dev_dbg(dev, "mul %u / div %u\n", mul, div); > > > > min_pre_pll_clk_div =
On Wed 2017-02-15 00:05:03, Sakari Ailus wrote: > Hi Pavel, > > On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote: > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > Required added multiplier (and divisor) calculation did not take into > > account the existing divisor when checking the values against the > > minimum divisor. Do just that. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > I need to understand again why did I write this patch. :-) Can you just trust your former self? > Could you send me the smiapp driver output with debug level messages > enabled, please? > I think the problem was with the secondary sensor. I believe it was with the main sensor, actually. Anyway, here are the messages. [ 0.791290] smiapp 2-0010: could not get clock (-517) [ 2.705352] smiapp 2-0010: GPIO lookup for consumer xshutdown [ 2.705352] smiapp 2-0010: using device tree for GPIO lookup [ 2.705413] smiapp 2-0010: using lookup tables for GPIO lookup [ 2.705413] smiapp 2-0010: lookup for GPIO xshutdown failed [ 2.875244] smiapp 2-0010: lane_op_clock_ratio: 1 [ 2.875274] smiapp 2-0010: binning: 1x1 [ 2.875274] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 [ 2.875305] smiapp 2-0010: pre-pll check: min / max pre_pll_clk_div: 1 / 1 [ 2.875305] smiapp 2-0010: mul 25 / div 2 [ 2.875305] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: 1 / 1 [ 2.875335] smiapp 2-0010: pre_pll_clk_div 1 [ 2.875335] smiapp 2-0010: more_mul_max: max_pll_multiplier check: 1 [ 2.875335] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: 1 [ 2.875335] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: 1 [ 2.875366] smiapp 2-0010: more_mul_max: min_pll_multiplier check: 1 [ 2.875366] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: 1 [ 2.875366] smiapp 2-0010: more_mul_min: min_pll_multiplier check: 1 [ 2.875396] smiapp 2-0010: more_mul_factor: 1 [ 2.875396] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 [ 2.875396] smiapp 2-0010: final more_mul: 1 [ 2.875427] smiapp 2-0010: op_sys_clk_div: 2 [ 2.875427] smiapp 2-0010: op_pix_clk_div: 10 [ 2.875427] smiapp 2-0010: pre_pll_clk_div 1 [ 2.875457] smiapp 2-0010: pll_multiplier 25 [ 2.875457] smiapp 2-0010: vt_sys_clk_div 2 [ 2.875457] smiapp 2-0010: vt_pix_clk_div 10 [ 2.875457] smiapp 2-0010: ext_clk_freq_hz 9600000 [ 2.875488] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 [ 2.875488] smiapp 2-0010: pll_op_clk_freq_hz 240000000 [ 2.875488] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 [ 2.875518] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 [ 2.876068] smiapp 2-0010: lane_op_clock_ratio: 1 [ 2.876068] smiapp 2-0010: binning: 1x1 [ 2.876098] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 [ 2.876098] smiapp 2-0010: pre-pll check: min / max pre_pll_clk_div: 1 / 1 [ 2.876098] smiapp 2-0010: mul 25 / div 2 [ 2.876129] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: 1 / 1 [ 2.876129] smiapp 2-0010: pre_pll_clk_div 1 [ 2.876129] smiapp 2-0010: more_mul_max: max_pll_multiplier check: 1 [ 2.876159] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: 1 [ 2.876159] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: 1 [ 2.876159] smiapp 2-0010: more_mul_max: min_pll_multiplier check: 1 [ 2.876190] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: 1 [ 2.876190] smiapp 2-0010: more_mul_min: min_pll_multiplier check: 1 [ 2.876190] smiapp 2-0010: more_mul_factor: 1 [ 2.876190] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 [ 2.876220] smiapp 2-0010: final more_mul: 1 [ 2.876220] smiapp 2-0010: op_sys_clk_div: 2 [ 2.876220] smiapp 2-0010: op_pix_clk_div: 10 [ 2.876251] smiapp 2-0010: pre_pll_clk_div 1 [ 2.876251] smiapp 2-0010: pll_multiplier 25 [ 2.876251] smiapp 2-0010: vt_sys_clk_div 2 [ 2.876251] smiapp 2-0010: vt_pix_clk_div 10 [ 2.876281] smiapp 2-0010: ext_clk_freq_hz 9600000 [ 2.876281] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 [ 2.876281] smiapp 2-0010: pll_op_clk_freq_hz 240000000 [ 2.876312] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 [ 2.876312] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 ... [ 4.728973] udevd[216]: starting version 175 [ 8.031494] smiapp 2-0010: lane_op_clock_ratio: 1 [ 8.031524] smiapp 2-0010: binning: 1x1 [ 8.031524] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 [ 8.031524] smiapp 2-0010: pre-pll check: min / max pre_pll_clk_div: 1 / 1 [ 8.031555] smiapp 2-0010: mul 25 / div 2 [ 8.031555] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: 1 / 1 [ 8.031555] smiapp 2-0010: pre_pll_clk_div 1 [ 8.031585] smiapp 2-0010: more_mul_max: max_pll_multiplier check: 1 [ 8.031585] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: 1 [ 8.031585] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: 1 [ 8.031616] smiapp 2-0010: more_mul_max: min_pll_multiplier check: 1 [ 8.031616] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: 1 [ 8.031616] smiapp 2-0010: more_mul_min: min_pll_multiplier check: 1 [ 8.031616] smiapp 2-0010: more_mul_factor: 1 [ 8.031646] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 [ 8.031646] smiapp 2-0010: final more_mul: 1 [ 8.031646] smiapp 2-0010: op_sys_clk_div: 2 [ 8.031677] smiapp 2-0010: op_pix_clk_div: 10 [ 8.031677] smiapp 2-0010: pre_pll_clk_div 1 [ 8.031677] smiapp 2-0010: pll_multiplier 25 [ 8.031707] smiapp 2-0010: vt_sys_clk_div 2 [ 8.031707] smiapp 2-0010: vt_pix_clk_div 10 [ 8.031707] smiapp 2-0010: ext_clk_freq_hz 9600000 [ 8.031738] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 [ 8.031738] smiapp 2-0010: pll_op_clk_freq_hz 240000000 [ 8.031738] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 [ 8.031768] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 [ 8.064117] smiapp 2-0010: lane_op_clock_ratio: 1 [ 8.064147] smiapp 2-0010: binning: 1x1 [ 8.064147] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 [ 8.064178] smiapp 2-0010: pre-pll check: min / max pre_pll_clk_div: 1 / 1 [ 8.064178] smiapp 2-0010: mul 25 / div 2 [ 8.064178] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: 1 / 1 [ 8.064208] smiapp 2-0010: pre_pll_clk_div 1 [ 8.064208] smiapp 2-0010: more_mul_max: max_pll_multiplier check: 1 [ 8.064208] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: 1 [ 8.064239] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: 1 [ 8.064239] smiapp 2-0010: more_mul_max: min_pll_multiplier check: 1 [ 8.064239] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: 1 [ 8.064239] smiapp 2-0010: more_mul_min: min_pll_multiplier check: 1 [ 8.064270] smiapp 2-0010: more_mul_factor: 1 [ 8.064270] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 [ 8.064270] smiapp 2-0010: final more_mul: 1 [ 8.064300] smiapp 2-0010: op_sys_clk_div: 2 [ 8.064300] smiapp 2-0010: op_pix_clk_div: 10 [ 8.064300] smiapp 2-0010: pre_pll_clk_div 1 [ 8.064331] smiapp 2-0010: pll_multiplier 25 [ 8.064331] smiapp 2-0010: vt_sys_clk_div 2 [ 8.064331] smiapp 2-0010: vt_pix_clk_div 10 [ 8.064361] smiapp 2-0010: ext_clk_freq_hz 9600000 [ 8.064361] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 [ 8.064361] smiapp 2-0010: pll_op_clk_freq_hz 240000000 [ 8.064392] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 [ 8.064392] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 Best regards, Pavel
On Wednesday 15 February 2017 00:05:03 Sakari Ailus wrote: > Hi Pavel, > > On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote: > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > Required added multiplier (and divisor) calculation did not take into > > account the existing divisor when checking the values against the > > minimum divisor. Do just that. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > I need to understand again why did I write this patch. :-) > > Could you send me the smiapp driver output with debug level messages > enabled, please? > > I think the problem was with the secondary sensor. > Hi, search for emails and threads: Message-Id: <1364719448-29894-1-git-send-email-sakari.ailus@iki.fi> Message-ID: <5728ED34.3060402@gmail.com> I think I already resent those information again :-)
Hi! > > On Tue, Feb 14, 2017 at 02:40:04PM +0100, Pavel Machek wrote: > > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > > > Required added multiplier (and divisor) calculation did not take into > > > account the existing divisor when checking the values against the > > > minimum divisor. Do just that. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > > > I need to understand again why did I write this patch. :-) > > Can you just trust your former self? > > > Could you send me the smiapp driver output with debug level messages > > enabled, please? > > > I think the problem was with the secondary sensor. > > I believe it was with the main sensor, actually. Anyway, here are the > messages. Can I get you to apply this one? :-). Thanks, Pavel > [ 0.791290] smiapp 2-0010: could not get clock (-517) > [ 2.705352] smiapp 2-0010: GPIO lookup for consumer xshutdown > [ 2.705352] smiapp 2-0010: using device tree for GPIO lookup > [ 2.705413] smiapp 2-0010: using lookup tables for GPIO lookup > [ 2.705413] smiapp 2-0010: lookup for GPIO xshutdown failed > [ 2.875244] smiapp 2-0010: lane_op_clock_ratio: 1 > [ 2.875274] smiapp 2-0010: binning: 1x1 > [ 2.875274] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 > [ 2.875305] smiapp 2-0010: pre-pll check: min / max > pre_pll_clk_div: 1 / 1 > [ 2.875305] smiapp 2-0010: mul 25 / div 2 > [ 2.875305] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: > 1 / 1 > [ 2.875335] smiapp 2-0010: pre_pll_clk_div 1 > [ 2.875335] smiapp 2-0010: more_mul_max: max_pll_multiplier check: > 1 > [ 2.875335] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: > 1 > [ 2.875335] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: > 1 > [ 2.875366] smiapp 2-0010: more_mul_max: min_pll_multiplier check: > 1 > [ 2.875366] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: > 1 > [ 2.875366] smiapp 2-0010: more_mul_min: min_pll_multiplier check: > 1 > [ 2.875396] smiapp 2-0010: more_mul_factor: 1 > [ 2.875396] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 > [ 2.875396] smiapp 2-0010: final more_mul: 1 > [ 2.875427] smiapp 2-0010: op_sys_clk_div: 2 > [ 2.875427] smiapp 2-0010: op_pix_clk_div: 10 > [ 2.875427] smiapp 2-0010: pre_pll_clk_div 1 > [ 2.875457] smiapp 2-0010: pll_multiplier 25 > [ 2.875457] smiapp 2-0010: vt_sys_clk_div 2 > [ 2.875457] smiapp 2-0010: vt_pix_clk_div 10 > [ 2.875457] smiapp 2-0010: ext_clk_freq_hz 9600000 > [ 2.875488] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 > [ 2.875488] smiapp 2-0010: pll_op_clk_freq_hz 240000000 > [ 2.875488] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 > [ 2.875518] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 > [ 2.876068] smiapp 2-0010: lane_op_clock_ratio: 1 > [ 2.876068] smiapp 2-0010: binning: 1x1 > [ 2.876098] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 > [ 2.876098] smiapp 2-0010: pre-pll check: min / max > pre_pll_clk_div: 1 / 1 > [ 2.876098] smiapp 2-0010: mul 25 / div 2 > [ 2.876129] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: > 1 / 1 > [ 2.876129] smiapp 2-0010: pre_pll_clk_div 1 > [ 2.876129] smiapp 2-0010: more_mul_max: max_pll_multiplier check: > 1 > [ 2.876159] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: > 1 > [ 2.876159] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: > 1 > [ 2.876159] smiapp 2-0010: more_mul_max: min_pll_multiplier check: > 1 > [ 2.876190] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: > 1 > [ 2.876190] smiapp 2-0010: more_mul_min: min_pll_multiplier check: > 1 > [ 2.876190] smiapp 2-0010: more_mul_factor: 1 > [ 2.876190] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 > [ 2.876220] smiapp 2-0010: final more_mul: 1 > [ 2.876220] smiapp 2-0010: op_sys_clk_div: 2 > [ 2.876220] smiapp 2-0010: op_pix_clk_div: 10 > [ 2.876251] smiapp 2-0010: pre_pll_clk_div 1 > [ 2.876251] smiapp 2-0010: pll_multiplier 25 > [ 2.876251] smiapp 2-0010: vt_sys_clk_div 2 > [ 2.876251] smiapp 2-0010: vt_pix_clk_div 10 > [ 2.876281] smiapp 2-0010: ext_clk_freq_hz 9600000 > [ 2.876281] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 > [ 2.876281] smiapp 2-0010: pll_op_clk_freq_hz 240000000 > [ 2.876312] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 > [ 2.876312] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 > ... > [ 4.728973] udevd[216]: starting version 175 > [ 8.031494] smiapp 2-0010: lane_op_clock_ratio: 1 > [ 8.031524] smiapp 2-0010: binning: 1x1 > [ 8.031524] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 > [ 8.031524] smiapp 2-0010: pre-pll check: min / max > pre_pll_clk_div: 1 / 1 > [ 8.031555] smiapp 2-0010: mul 25 / div 2 > [ 8.031555] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: > 1 / 1 > [ 8.031555] smiapp 2-0010: pre_pll_clk_div 1 > [ 8.031585] smiapp 2-0010: more_mul_max: max_pll_multiplier check: > 1 > [ 8.031585] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: > 1 > [ 8.031585] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: > 1 > [ 8.031616] smiapp 2-0010: more_mul_max: min_pll_multiplier check: > 1 > [ 8.031616] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: > 1 > [ 8.031616] smiapp 2-0010: more_mul_min: min_pll_multiplier check: > 1 > [ 8.031616] smiapp 2-0010: more_mul_factor: 1 > [ 8.031646] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 > [ 8.031646] smiapp 2-0010: final more_mul: 1 > [ 8.031646] smiapp 2-0010: op_sys_clk_div: 2 > [ 8.031677] smiapp 2-0010: op_pix_clk_div: 10 > [ 8.031677] smiapp 2-0010: pre_pll_clk_div 1 > [ 8.031677] smiapp 2-0010: pll_multiplier 25 > [ 8.031707] smiapp 2-0010: vt_sys_clk_div 2 > [ 8.031707] smiapp 2-0010: vt_pix_clk_div 10 > [ 8.031707] smiapp 2-0010: ext_clk_freq_hz 9600000 > [ 8.031738] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 > [ 8.031738] smiapp 2-0010: pll_op_clk_freq_hz 240000000 > [ 8.031738] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 > [ 8.031768] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 > [ 8.064117] smiapp 2-0010: lane_op_clock_ratio: 1 > [ 8.064147] smiapp 2-0010: binning: 1x1 > [ 8.064147] smiapp 2-0010: min / max pre_pll_clk_div: 1 / 4 > [ 8.064178] smiapp 2-0010: pre-pll check: min / max > pre_pll_clk_div: 1 / 1 > [ 8.064178] smiapp 2-0010: mul 25 / div 2 > [ 8.064178] smiapp 2-0010: pll_op check: min / max pre_pll_clk_div: > 1 / 1 > [ 8.064208] smiapp 2-0010: pre_pll_clk_div 1 > [ 8.064208] smiapp 2-0010: more_mul_max: max_pll_multiplier check: > 1 > [ 8.064208] smiapp 2-0010: more_mul_max: max_pll_op_freq_hz check: > 1 > [ 8.064239] smiapp 2-0010: more_mul_max: max_op_sys_clk_div check: > 1 > [ 8.064239] smiapp 2-0010: more_mul_max: min_pll_multiplier check: > 1 > [ 8.064239] smiapp 2-0010: more_mul_min: min_pll_op_freq_hz check: > 1 > [ 8.064239] smiapp 2-0010: more_mul_min: min_pll_multiplier check: > 1 > [ 8.064270] smiapp 2-0010: more_mul_factor: 1 > [ 8.064270] smiapp 2-0010: more_mul_factor: min_op_sys_clk_div: 1 > [ 8.064270] smiapp 2-0010: final more_mul: 1 > [ 8.064300] smiapp 2-0010: op_sys_clk_div: 2 > [ 8.064300] smiapp 2-0010: op_pix_clk_div: 10 > [ 8.064300] smiapp 2-0010: pre_pll_clk_div 1 > [ 8.064331] smiapp 2-0010: pll_multiplier 25 > [ 8.064331] smiapp 2-0010: vt_sys_clk_div 2 > [ 8.064331] smiapp 2-0010: vt_pix_clk_div 10 > [ 8.064361] smiapp 2-0010: ext_clk_freq_hz 9600000 > [ 8.064361] smiapp 2-0010: pll_ip_clk_freq_hz 9600000 > [ 8.064361] smiapp 2-0010: pll_op_clk_freq_hz 240000000 > [ 8.064392] smiapp 2-0010: vt_sys_clk_freq_hz 120000000 > [ 8.064392] smiapp 2-0010: vt_pix_clk_freq_hz 12000000 > > Best regards, > Pavel > >
On Tue, Feb 28, 2017 at 03:09:21PM +0100, Pavel Machek wrote:
> Can I get you to apply this one? :-).
Let me try to understand again what does that change actually do. I'll find
the time during the rest of this week.
I'm starting to think we need a test suite for the PLL calculator...
On Tue 2017-02-28 16:16:21, Sakari Ailus wrote: > On Tue, Feb 28, 2017 at 03:09:21PM +0100, Pavel Machek wrote: > > Can I get you to apply this one? :-). > > Let me try to understand again what does that change actually do. I'll find > the time during the rest of this week. > > I'm starting to think we need a test suite for the PLL calculator... Any update here or on the other patch? We are quite close to working camera now... Plus I have played with v4l-utils, and managed to implement autofocus and autoexposure -- it was easier than expected. I believe you mentioned you had some patches to automatically initialize the pipeline. Do you and can I have them? Last thing.. Is someone able to compute new modes for et8ek8? I believe smaller than 640x480 mode would be useful for video streaming, and I still can't get 5MPix mode to work; understanding what goes on there would be useful. Thanks, Pavel
On Thu 2017-03-23 00:46:51, Pavel Machek wrote: > On Tue 2017-02-28 16:16:21, Sakari Ailus wrote: > > On Tue, Feb 28, 2017 at 03:09:21PM +0100, Pavel Machek wrote: > > > Can I get you to apply this one? :-). > > > > Let me try to understand again what does that change actually do. I'll find > > the time during the rest of this week. > > > > I'm starting to think we need a test suite for the PLL calculator... > > Any update here or on the other patch? We are quite close to working > camera now... > > Plus I have played with v4l-utils, and managed to implement autofocus > and autoexposure -- it was easier than expected. I believe you > mentioned you had some patches to automatically initialize the > pipeline. Do you and can I have them? > > Last thing.. Is someone able to compute new modes for et8ek8? I > believe smaller than 640x480 mode would be useful for video streaming, > and I still can't get 5MPix mode to work; understanding what goes on > there would be useful. Oh and.. is it possible to generate modes with more than 30fps? I guess that would be useful for initial autofocus/autogain.... Pavel
Hi Pavel, On Thu, Mar 23, 2017 at 12:46:51AM +0100, Pavel Machek wrote: > On Tue 2017-02-28 16:16:21, Sakari Ailus wrote: > > On Tue, Feb 28, 2017 at 03:09:21PM +0100, Pavel Machek wrote: > > > Can I get you to apply this one? :-). > > > > Let me try to understand again what does that change actually do. I'll find > > the time during the rest of this week. > > > > I'm starting to think we need a test suite for the PLL calculator... > > Any update here or on the other patch? We are quite close to working > camera now... I've been working on PLL test cases. The PLL calculator really requires that to be able to test any changes made to it. > > Plus I have played with v4l-utils, and managed to implement autofocus > and autoexposure -- it was easier than expected. I believe you > mentioned you had some patches to automatically initialize the > pipeline. Do you and can I have them? It was an early prototype and it wasn't really functional yet. Given a video node, it can find possible pipelines to the image sources with common formats. I.e. the ccdc -> rsz path is not available for raw cameras. C (especially without helper libraries) wasn't particularly suitable for the task, the data structures I had didn't end up too nice. What would also be necessary is to associate library or application specific data to entities, this could be as simple as key-value pairs with both key and value being pointers. > > Last thing.. Is someone able to compute new modes for et8ek8? I > believe smaller than 640x480 mode would be useful for video streaming, > and I still can't get 5MPix mode to work; understanding what goes on > there would be useful. Unfortunately the et8ek8 does not conform to a standard such as SMIA. :-( I'm not sure the datasheet provides enough information to come up with new mode definitions. Perhaps with some experimentation it could be possible. There are a few additional embedded documents in it, those are worth checking out. LibreOffice can open them AFAIR.
Hi! > > Plus I have played with v4l-utils, and managed to implement autofocus > > and autoexposure -- it was easier than expected. I believe you > > mentioned you had some patches to automatically initialize the > > pipeline. Do you and can I have them? > > It was an early prototype and it wasn't really functional yet. > > Given a video node, it can find possible pipelines to the image sources with > common formats. I.e. the ccdc -> rsz path is not available for raw > cameras. > C (especially without helper libraries) wasn't particularly suitable for the > task, the data structures I had didn't end up too nice. What would also be > necessary is to associate library or application specific data to entities, > this could be as simple as key-value pairs with both key and value being > pointers. Could I get a copy, anyway? Need not be perfect, but starting point would be welcome. Thanks, Pavel
diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index 771db56..166bbaf 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -16,6 +16,8 @@ * General Public License for more details. */ +#define DEBUG + #include <linux/device.h> #include <linux/gcd.h> #include <linux/lcm.h> @@ -227,7 +229,8 @@ static int __smiapp_pll_calculate( more_mul_factor = lcm(div, pll->pre_pll_clk_div) / div; dev_dbg(dev, "more_mul_factor: %u\n", more_mul_factor); - more_mul_factor = lcm(more_mul_factor, op_limits->min_sys_clk_div); + more_mul_factor = lcm(more_mul_factor, + DIV_ROUND_UP(op_limits->min_sys_clk_div, div)); dev_dbg(dev, "more_mul_factor: min_op_sys_clk_div: %d\n", more_mul_factor); i = roundup(more_mul_min, more_mul_factor); @@ -456,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev, i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz); mul = div_u64(pll->pll_op_clk_freq_hz, i); div = pll->ext_clk_freq_hz / i; + if (!mul) { + dev_err(dev, "forcing mul to 1\n"); + mul = 1; + } dev_dbg(dev, "mul %u / div %u\n", mul, div); min_pre_pll_clk_div =