From: "Roman Beranek" <roman.beranek@prusa3d.cz> To: "Emil Lenngren" <emil.lenngren@gmail.com> Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>, "Thierry Reding" <thierry.reding@gmail.com>, "Lee Jones" <lee.jones@linaro.org>, "Maxime Ripard" <mripard@kernel.org>, "Chen-Yu Tsai" <wens@csie.org>, "Jernej Skrabec" <jernej.skrabec@siol.net>, linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-sunxi@googlegroups.com, "Roman Beranek" <roman.beranek@prusa3d.com> Subject: Re: [PATCH] pwm: sun4i: Avoid waiting until the next period Date: Wed, 12 May 2021 06:13:36 +0200 [thread overview] Message-ID: <CBAZ3OSXC4KP.2VOXZAM4BL2F3@zen.local> (raw) In-Reply-To: <CAO1O6seU7t==O=yCVBQK0iAkeEyO3dbRQ71obJh3Jm26xxWobw@mail.gmail.com> Hello Emil, On Wed May 12, 2021 at 2:55 AM CEST, Emil Lenngren wrote: > But on what hardware do you really need to wait until one full pulse > cycle ends, before a disable command takes effect? I have no idea. The value has been there already for nearly 4 years (since c32c5c50d4fe). > By closing the gate when the pwm should be disabled, I guess we could > save some nanoampere or microampere (is this important?) My guess is that once the last cycle ends, the counter won't get incremented any longer. But my guess is of course as good as yours, I don't have an easy access to equipment capable of measurement this precise. > On the hardware I've tested on (GR8 and V3s), it's enough to wait at > most two clock cycles in order for it to take effect before we can > close the gate. And with clock cycle I mean 24 MHz divided by the > prescaler. With prescaler 1, that's 84 nanoseconds. In such case I could easily imagine keeping the clock gate around. Like this: --- drivers/pwm/pwm-sun4i.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index d6d6d43f6e81..3350f6517dbd 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -73,7 +73,7 @@ static const u32 prescaler_table[] = { 72000, 0, 0, - 0, /* Actually 1 but tested separately */ + 1, /* Tested separately */ }; struct sun4i_pwm_data { @@ -91,7 +91,7 @@ struct sun4i_pwm_chip { void __iomem *base; spinlock_t ctrl_lock; const struct sun4i_pwm_data *data; - unsigned long next_period[2]; + u64 ready_to_be_gated[2]; }; static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip) @@ -237,10 +237,12 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state cstate; u32 ctrl, duty = 0, period = 0, val; int ret; - unsigned int delay_us, prescaler = 0; - unsigned long now; + unsigned int cycle_ns, delay, prescaler = 0; + u64 now; bool bypass; + cycle_ns = NSEC_PER_SEC / clk_get_rate(sun4i_pwm->clk); + pwm_get_state(pwm, &cstate); if (!cstate.enabled) { @@ -286,8 +288,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, val = (duty & PWM_DTY_MASK) | PWM_PRD(period); sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm)); - sun4i_pwm->next_period[pwm->hwpwm] = jiffies + - nsecs_to_jiffies(cstate.period + 1000); + + now = get_jiffies_64(); + delay = nsecs_to_jiffies(2 * prescaler_table[prescaler] * cycle_ns) + 1; + if (time_before64(sun4i_pwm->ready_to_be_gated[pwm->hwpwm], now + delay)) + sun4i_pwm->ready_to_be_gated[pwm->hwpwm] = now + delay; if (state->polarity != PWM_POLARITY_NORMAL) ctrl &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm); @@ -298,6 +303,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (state->enabled) ctrl |= BIT_CH(PWM_EN, pwm->hwpwm); + else + ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm); sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); @@ -306,21 +313,15 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (state->enabled) return 0; - /* We need a full period to elapse before disabling the channel. */ - now = jiffies; - if (time_before(now, sun4i_pwm->next_period[pwm->hwpwm])) { - delay_us = jiffies_to_usecs(sun4i_pwm->next_period[pwm->hwpwm] - - now); - if ((delay_us / 500) > MAX_UDELAY_MS) - msleep(delay_us / 1000 + 1); - else - usleep_range(delay_us, delay_us * 2); + /* We need 1-2 clock cycles to elapse before disabling the channel. */ + if (time_before64(now, sun4i_pwm->ready_to_be_gated[pwm->hwpwm])) { + delay = (unsigned int)(sun4i_pwm->ready_to_be_gated[pwm->hwpwm] - now); + msleep(jiffies_to_msecs(delay)); } spin_lock(&sun4i_pwm->ctrl_lock); ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); - ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm); sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); spin_unlock(&sun4i_pwm->ctrl_lock);
WARNING: multiple messages have this Message-ID (diff)
From: "Roman Beranek" <roman.beranek@prusa3d.cz> To: "Emil Lenngren" <emil.lenngren@gmail.com> Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>, "Thierry Reding" <thierry.reding@gmail.com>, "Lee Jones" <lee.jones@linaro.org>, "Maxime Ripard" <mripard@kernel.org>, "Chen-Yu Tsai" <wens@csie.org>, "Jernej Skrabec" <jernej.skrabec@siol.net>, linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-sunxi@googlegroups.com, "Roman Beranek" <roman.beranek@prusa3d.com> Subject: Re: [PATCH] pwm: sun4i: Avoid waiting until the next period Date: Wed, 12 May 2021 06:13:36 +0200 [thread overview] Message-ID: <CBAZ3OSXC4KP.2VOXZAM4BL2F3@zen.local> (raw) In-Reply-To: <CAO1O6seU7t==O=yCVBQK0iAkeEyO3dbRQ71obJh3Jm26xxWobw@mail.gmail.com> Hello Emil, On Wed May 12, 2021 at 2:55 AM CEST, Emil Lenngren wrote: > But on what hardware do you really need to wait until one full pulse > cycle ends, before a disable command takes effect? I have no idea. The value has been there already for nearly 4 years (since c32c5c50d4fe). > By closing the gate when the pwm should be disabled, I guess we could > save some nanoampere or microampere (is this important?) My guess is that once the last cycle ends, the counter won't get incremented any longer. But my guess is of course as good as yours, I don't have an easy access to equipment capable of measurement this precise. > On the hardware I've tested on (GR8 and V3s), it's enough to wait at > most two clock cycles in order for it to take effect before we can > close the gate. And with clock cycle I mean 24 MHz divided by the > prescaler. With prescaler 1, that's 84 nanoseconds. In such case I could easily imagine keeping the clock gate around. Like this: --- drivers/pwm/pwm-sun4i.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index d6d6d43f6e81..3350f6517dbd 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -73,7 +73,7 @@ static const u32 prescaler_table[] = { 72000, 0, 0, - 0, /* Actually 1 but tested separately */ + 1, /* Tested separately */ }; struct sun4i_pwm_data { @@ -91,7 +91,7 @@ struct sun4i_pwm_chip { void __iomem *base; spinlock_t ctrl_lock; const struct sun4i_pwm_data *data; - unsigned long next_period[2]; + u64 ready_to_be_gated[2]; }; static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip) @@ -237,10 +237,12 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state cstate; u32 ctrl, duty = 0, period = 0, val; int ret; - unsigned int delay_us, prescaler = 0; - unsigned long now; + unsigned int cycle_ns, delay, prescaler = 0; + u64 now; bool bypass; + cycle_ns = NSEC_PER_SEC / clk_get_rate(sun4i_pwm->clk); + pwm_get_state(pwm, &cstate); if (!cstate.enabled) { @@ -286,8 +288,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, val = (duty & PWM_DTY_MASK) | PWM_PRD(period); sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm)); - sun4i_pwm->next_period[pwm->hwpwm] = jiffies + - nsecs_to_jiffies(cstate.period + 1000); + + now = get_jiffies_64(); + delay = nsecs_to_jiffies(2 * prescaler_table[prescaler] * cycle_ns) + 1; + if (time_before64(sun4i_pwm->ready_to_be_gated[pwm->hwpwm], now + delay)) + sun4i_pwm->ready_to_be_gated[pwm->hwpwm] = now + delay; if (state->polarity != PWM_POLARITY_NORMAL) ctrl &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm); @@ -298,6 +303,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (state->enabled) ctrl |= BIT_CH(PWM_EN, pwm->hwpwm); + else + ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm); sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); @@ -306,21 +313,15 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (state->enabled) return 0; - /* We need a full period to elapse before disabling the channel. */ - now = jiffies; - if (time_before(now, sun4i_pwm->next_period[pwm->hwpwm])) { - delay_us = jiffies_to_usecs(sun4i_pwm->next_period[pwm->hwpwm] - - now); - if ((delay_us / 500) > MAX_UDELAY_MS) - msleep(delay_us / 1000 + 1); - else - usleep_range(delay_us, delay_us * 2); + /* We need 1-2 clock cycles to elapse before disabling the channel. */ + if (time_before64(now, sun4i_pwm->ready_to_be_gated[pwm->hwpwm])) { + delay = (unsigned int)(sun4i_pwm->ready_to_be_gated[pwm->hwpwm] - now); + msleep(jiffies_to_msecs(delay)); } spin_lock(&sun4i_pwm->ctrl_lock); ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); - ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm); sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG); spin_unlock(&sun4i_pwm->ctrl_lock); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-12 5:01 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-11 22:00 [PATCH] pwm: sun4i: Avoid waiting until the next period Roman Beranek 2021-05-11 22:00 ` Roman Beranek 2021-05-12 0:55 ` Emil Lenngren 2021-05-12 0:55 ` Emil Lenngren 2021-05-12 4:13 ` Roman Beranek [this message] 2021-05-12 4:13 ` Roman Beranek 2021-05-12 4:41 ` Uwe Kleine-König 2021-05-12 4:41 ` Uwe Kleine-König 2021-05-12 9:18 ` Emil Lenngren 2021-05-12 9:18 ` Emil Lenngren 2021-05-12 9:18 ` Emil Lenngren 2021-05-25 16:41 ` Thierry Reding 2021-05-25 16:41 ` Thierry Reding 2021-05-27 12:10 ` Roman Beranek 2021-05-27 12:10 ` Roman Beranek 2021-05-27 13:53 ` Alexandre Belloni 2021-05-27 13:53 ` Alexandre Belloni 2021-06-25 17:25 ` Uwe Kleine-König 2021-06-25 17:25 ` Uwe Kleine-König 2021-05-12 5:31 ` Roman Beranek 2021-05-12 5:31 ` Roman Beranek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CBAZ3OSXC4KP.2VOXZAM4BL2F3@zen.local \ --to=roman.beranek@prusa3d.cz \ --cc=emil.lenngren@gmail.com \ --cc=jernej.skrabec@siol.net \ --cc=lee.jones@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pwm@vger.kernel.org \ --cc=linux-sunxi@googlegroups.com \ --cc=linux-sunxi@lists.linux.dev \ --cc=mripard@kernel.org \ --cc=roman.beranek@prusa3d.com \ --cc=thierry.reding@gmail.com \ --cc=u.kleine-koenig@pengutronix.de \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.