[RFC PATCH] pwm: pwm-samsung: trigger manual update on 100% duty

[RFC PATCH] pwm: pwm-samsung: trigger manual update on 100% duty

[Martin Jücker]

Hello,

I have some trouble with a pwm backlight in the p4note and I'm not sure
what the cause is. The problem is with setting the backlight brightness
to its maximum value, which will actually turn off the display. All the
other values seem to be working just fine.

Looking at the documentation for the pwm driver in the exynos4412
manual, I found that for pwm, the higher the tcmp value, the lower the
pwm value is. Enabling debug logging, this seems to be exactly what's
happening for values below 100% duty, but when the duty reaches 100%,
the tcmp is zero and then gets decreased which causes it to wrap around.
Looking at the rest of the code, this seems to be expected behaviour and
I would assume that it should cause the pwm to never switch the value
from high to low which would also fit.

Now I tried around and found two solutions to my problem, one of them is
attached to the mail and just triggers a manual update if the new value
wrapped around. This makes the display behave as expected but I cannot
explain what's actually happening. Another idea was to check the tcmp
value before decreasing to avoid the wrap around, but then 100% duty
cycle is not possible anymore as the pwm signal will switch from high to
low on the last tick of the counter.

I'd be happy for any input on this issue.

Here is the first solution I found:


Trigger a manual update to the pwm chip when going from any value to
100% duty to avoid the TOUT value staying low.

Signed-off-by: Martin Jücker <martin.juecker@gmail.com>
---
 drivers/pwm/pwm-samsung.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 0a4ff55fad04..a449e38a24da 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -383,7 +383,7 @@ static int __pwm_samsung_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * update to prevent the signal staying high if the PWM is disabled
 	 * shortly afer this update (before it autoreloaded the new values).
 	 */
-	if (oldtcmp == (u32) -1) {
+	if (oldtcmp == (u32)-1 || tcmp == (u32)-1) {
 		dev_dbg(our_chip->chip.dev, "Forcing manual update");
 		pwm_samsung_manual_update(our_chip, pwm);
 	}
-- 
2.25.1

Re: [RFC PATCH] pwm: pwm-samsung: trigger manual update on 100% duty

[Krzysztof Kozlowski]

On 09/04/2022 15:40, Martin Jücker wrote:
> Hello,
> 
> I have some trouble with a pwm backlight in the p4note and I'm not sure
> what the cause is. The problem is with setting the backlight brightness
> to its maximum value, which will actually turn off the display. All the
> other values seem to be working just fine.

A bit similar issues was fixed by Mårten (5d82e661398e ("pwm:
pwm-samsung: Trigger manual update when disabling PWM")
). I assume you test on a tree having that commit.

> 
> Looking at the documentation for the pwm driver in the exynos4412
> manual, I found that for pwm, the higher the tcmp value, the lower the
> pwm value is. Enabling debug logging, this seems to be exactly what's
> happening for values below 100% duty, but when the duty reaches 100%,
> the tcmp is zero and then gets decreased which causes it to wrap around.
> Looking at the rest of the code, this seems to be expected behaviour and
> I would assume that it should cause the pwm to never switch the value
> from high to low which would also fit.

I miss here some details.

The tcmp=-1 is written to registers for the next duty cycle. When
current duty cycle finishes, this new tcmp=-1 should be loaded into the PWM.

It seems that this loading does not happen in your case. Correct?

I tried this on Odroid HC1 and works correctly:
duty_ns=2000000, period_ns=2000000
tin_ns=240
tcmp=4294967295
tcnt=8332

What values do you have? How can I reproduce your case?

Best regards,
Krzysztof

Re: [RFC PATCH] pwm: pwm-samsung: trigger manual update on 100% duty

[Martin Jücker]

Hi Krzysztof,

On Sat, Apr 09, 2022 at 05:01:42PM +0200, Krzysztof Kozlowski wrote:
> On 09/04/2022 15:40, Martin Jücker wrote:
> > Hello,
> > 
> > I have some trouble with a pwm backlight in the p4note and I'm not sure
> > what the cause is. The problem is with setting the backlight brightness
> > to its maximum value, which will actually turn off the display. All the
> > other values seem to be working just fine.
> 
> A bit similar issues was fixed by Mårten (5d82e661398e ("pwm:
> pwm-samsung: Trigger manual update when disabling PWM")
> ). I assume you test on a tree having that commit.

I applied this to next, so yes I do have this one :-)

> 
> > 
> > Looking at the documentation for the pwm driver in the exynos4412
> > manual, I found that for pwm, the higher the tcmp value, the lower the
> > pwm value is. Enabling debug logging, this seems to be exactly what's
> > happening for values below 100% duty, but when the duty reaches 100%,
> > the tcmp is zero and then gets decreased which causes it to wrap around.
> > Looking at the rest of the code, this seems to be expected behaviour and
> > I would assume that it should cause the pwm to never switch the value
> > from high to low which would also fit.
> 
> I miss here some details.
> 
> The tcmp=-1 is written to registers for the next duty cycle. When
> current duty cycle finishes, this new tcmp=-1 should be loaded into the PWM.
> 
> It seems that this loading does not happen in your case. Correct?

When I change to maximum brightness, the pwm does trigger an update. It
writes the value to the register, but for some reason this causes the
display to turn off on the next cycle instead of going to full
brightness.

> 
> I tried this on Odroid HC1 and works correctly:
> duty_ns=2000000, period_ns=2000000
> tin_ns=240
> tcmp=4294967295
> tcnt=8332
> 
> What values do you have? How can I reproduce your case?

For me it's the following values:

duty_ns=78770, period_ns=78770
tin_ns=10
tcmp=4294967295
tcnt=7876

For the patch, this morning it's not fixing the issue anymore. I might
have messed up while trying around with the tcmp value yesterday night
by still having the wrap around guard in. I thought I double checked
that, apparently I didn't.

> 
> Best regards,
> Krzysztof

Kind Regards
Martin