Re: [PATCH] pwm: sun4i: Avoid waiting until the next period

["Roman Beranek" <roman.beranek@prusa3d.cz>]

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);