On Thu, Oct 17, 2019 at 12:59:20PM +0200, Michal Vokáč wrote:
On 17. 10. 19 11:48, Michal Vokáč wrote:
On 17. 10. 19 10:10, Uwe Kleine-König wrote:
A previous change in the pwm core (namely 01ccf903edd6 ("pwm: Let pwm_get_state() return the last implemented state")) changed the semantic of pwm_get_state() and disclosed an (as it seems) common problem in lowlevel PWM drivers. By not relying on the period and duty cycle being retrievable from a disabled PWM this type of problem is worked around.
Apart from this issue only calling the pwm_get_state/pwm_apply_state combo once is also more effective.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de
Hello,
There are now two reports about 01ccf903edd6 breaking a backlight. As far as I understand the problem this is a combination of the backend pwm driver yielding surprising results and the pwm-bl driver doing things more complicated than necessary.
So I guess this patch works around these problems. Still it would be interesting to find out the details in the imx driver that triggers the problem. So Adam, can you please instrument the pwm-imx27 driver to print *state at the beginning of pwm_imx27_apply() and the end of pwm_imx27_get_state() and provide the results?
Note I only compile tested this change.
Hi Uwe, I was just about to respond to the "pwm_bl on i.MX6Q broken on 5.4-RC1+" thread that I have a similar problem when you submitted this patch.
So here are my few cents:
Once again with updated and more detailed debug messages.
My setup is as follows:
- imx6dl-yapp4-draco with i.MX6Solo
- backlight is controlled with inverted PWM signal
- max brightness level = 32, default brightness level set to 32 in DT.
- Almost correct backlight behavior before 01ccf903edd6 ("pwm: Let pwm_get_state() return the last implemented state):
- System boots to userspace and backlight is enabled all the time from power up.
- $ dmesg | grep pwm_ [ 1.761546] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 5.012352] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0 [ 5.021143] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 0 [ 5.030182] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 1
$ cat brightness 32
$ echo 32 > brightness # nothing happens, max. brightness
$ echo 1 > brightness # backlight goes down to lowest level
[ 93.976354] pwm_imx27_apply: period: 500000, duty: 7843, polarity: 1, enabled: 1
- $ echo 0 > brightness # backlight goes up to max. level, this is # problem of the inverted PWM on i.MX we attempted # to solve some time ago.
[ 115.496350] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
- Backlight behavior on v5.4-rc3:
- System boots to userspace and backlight is enabled all the time from power up.
- $ dmesg | grep pwm_ [ 1.774071] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 5.003961] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0 [ 5.012649] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 5.021694] pwm_imx27_apply: period: 992970, duty: 992970, polarity: 0, enabled: 0 [ 5.030732] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 5.039643] pwm_imx27_apply: period: 992970, duty: 0, polarity: 0, enabled: 1 [ 5.049605] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 1
$ cat brightness 32
$ echo 32 > brightness # backlight goes down
[ 707.946970] pwm_imx27_apply: period: 992970, duty: 992970, polarity: 0, enabled: 1 [ 707.958551] pwm_imx27_get_state: period: 992970, duty: 992970, polarity: 0, enabled: 1
- $ echo 1 > brightness # backlight goes up to high level
[ 757.516845] pwm_imx27_apply: period: 992970, duty: 15576, polarity: 0, enabled: 1 [ 757.528438] pwm_imx27_get_state: period: 992970, duty: 15576, polarity: 0, enabled: 1
- $ echo 0 > brightness # backlight goes up to highest level
[ 783.386838] pwm_imx27_apply: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 783.398025] pwm_imx27_get_state: period: 496485, duty: 0, polarity: 0, enabled: 0So the behavior is clearly inverted to how it worked prior to 01ccf903edd6 with the weird exception that the initial brightness level 32 is not applied.
- Backlight behavior on v5.4-rc3 + this patch:
System boots with backlight enabled. In the middle of kernel boot backlight is disabled.
$ dmesg | grep state
[ 1.773099] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0 [ 5.002532] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0 [ 5.011263] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
Here is another corner case: pwm_imx27_apply doesn't even set polarity in hardware if .enabled is false. This way .polarity is lost as obviously the wrong setting is returned by pwm_get_state() later.
I didn't check in detail, but at least atmel, atmel-hlcdc, fsl-ftm, stm32 and stm32-lp seem to suffer from the same problem.
Best regards Uwe