[PATCH] backlight: pwm_bl: configure pwm only once per backlight toggle
Michal Vokáč
michal.vokac at ysoft.com
Fri Oct 18 09:36:36 UTC 2019
On 17. 10. 19 19:44, Adam Ford wrote:
> On Thu, Oct 17, 2019 at 12:13 PM Thierry Reding
> <thierry.reding at gmail.com> wrote:
>>
>> On Thu, Oct 17, 2019 at 12:07:21PM -0500, Adam Ford wrote:
>>> On Thu, Oct 17, 2019 at 10:14 AM Thierry Reding
>>> <thierry.reding at gmail.com> wrote:
>>>>
>>>> On Thu, Oct 17, 2019 at 03:58:25PM +0200, Michal Vokáč wrote:
>>>>> On 17. 10. 19 14:59, Thierry Reding wrote:
>>>>>> On Thu, Oct 17, 2019 at 02:09:17PM +0200, Uwe Kleine-König wrote:
>>>>>>> On Thu, Oct 17, 2019 at 01:11:31PM +0200, Thierry Reding wrote:
>>>>>>>> On Thu, Oct 17, 2019 at 12:11:16PM +0200, Uwe Kleine-König wrote:
>>>>>>>>> On Thu, Oct 17, 2019 at 11:48:08AM +0200, 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 at 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:
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>> 1. 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 state
>>>>>>>>>> [ 1.763381] get state end: -1811360608, enabled: 0
>>>>>>>>>
>>>>>>>>> What is -1811360608? When I wrote "print *state" above, I thought about
>>>>>>>>> something like:
>>>>>>>>>
>>>>>>>>> pr_info("%s: period: %u, duty: %u, polarity: %d, enabled: %d",
>>>>>>>>> __func__, state->period, state->duty_cycle, state->polarity, state->enabled);
>>>>>>>>>
>>>>>>>>> A quick look into drivers/pwm/pwm-imx27.c shows that this is another
>>>>>>>>> driver that yields duty_cycle = 0 when the hardware is off.
>>>>>>>>
>>>>>>>> It seems to me like the best recourse to fix this for now would be to
>>>>>>>> patch up the drivers that return 0 when the hardware is off by caching
>>>>>>>> the currently configured duty cycle.
>>>>>>>>
>>>>>>>> How about the patch below?
>>>>>>>>
>>>>>>>> Thierry
>>>>>>>>
>>>>>>>> --- >8 ---
>>>>>>>> From 15a52a7f1b910804fabd74a5882befd3f9d6bb37 Mon Sep 17 00:00:00 2001
>>>>>>>> From: Thierry Reding <thierry.reding at gmail.com>
>>>>>>>> Date: Thu, 17 Oct 2019 12:56:00 +0200
>>>>>>>> Subject: [PATCH] pwm: imx27: Cache duty cycle register value
>>>>>>>>
>>>>>>>> The hardware register containing the duty cycle value cannot be accessed
>>>>>>>> when the PWM is disabled. This causes the ->get_state() callback to read
>>>>>>>> back a duty cycle value of 0, which can confuse consumer drivers.
>>>>>>>>
>>>>>>>> Signed-off-by: Thierry Reding <thierry.reding at gmail.com>
>>>>>>>> ---
>>>>>>>> drivers/pwm/pwm-imx27.c | 31 ++++++++++++++++++++++++-------
>>>>>>>> 1 file changed, 24 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
>>>>>>>> index ae11d8577f18..4113d5cd4c62 100644
>>>>>>>> --- a/drivers/pwm/pwm-imx27.c
>>>>>>>> +++ b/drivers/pwm/pwm-imx27.c
>>>>>>>> @@ -85,6 +85,13 @@ struct pwm_imx27_chip {
>>>>>>>> struct clk *clk_per;
>>>>>>>> void __iomem *mmio_base;
>>>>>>>> struct pwm_chip chip;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * The driver cannot read the current duty cycle from the hardware if
>>>>>>>> + * the hardware is disabled. Cache the last programmed duty cycle
>>>>>>>> + * value to return in that case.
>>>>>>>> + */
>>>>>>>> + unsigned int duty_cycle;
>>>>>>>> };
>>>>>>>> #define to_pwm_imx27_chip(chip) container_of(chip, struct pwm_imx27_chip, chip)
>>>>>>>> @@ -155,14 +162,17 @@ static void pwm_imx27_get_state(struct pwm_chip *chip,
>>>>>>>> tmp = NSEC_PER_SEC * (u64)(period + 2);
>>>>>>>> state->period = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk);
>>>>>>>> - /* PWMSAR can be read only if PWM is enabled */
>>>>>>>> - if (state->enabled) {
>>>>>>>> + /*
>>>>>>>> + * PWMSAR can be read only if PWM is enabled. If the PWM is disabled,
>>>>>>>> + * use the cached value.
>>>>>>>> + */
>>>>>>>> + if (state->enabled)
>>>>>>>> val = readl(imx->mmio_base + MX3_PWMSAR);
>>>>>>>> - tmp = NSEC_PER_SEC * (u64)(val);
>>>>>>>> - state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk);
>>>>>>>> - } else {
>>>>>>>> - state->duty_cycle = 0;
>>>>>>>> - }
>>>>>>>> + else
>>>>>>>> + val = imx->duty_cycle;
>>>>>>>> +
>>>>>>>> + tmp = NSEC_PER_SEC * (u64)(val);
>>>>>>>> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk);
>>>>>>>> if (!state->enabled)
>>>>>>>> pwm_imx27_clk_disable_unprepare(chip);
>>>>>>>> @@ -261,6 +271,13 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>>>>>>>> writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
>>>>>>>> writel(period_cycles, imx->mmio_base + MX3_PWMPR);
>>>>>>>> + /*
>>>>>>>> + * Store the duty cycle for future reference in cases where
>>>>>>>> + * the MX3_PWMSAR register can't be read (i.e. when the PWM
>>>>>>>> + * is disabled).
>>>>>>>> + */
>>>>>>>> + imx->duty_cycle = duty_cycles;
>>>>>>>> +
>>>>>>>
>>>>>>> I wonder if it would be more sensible to do this in the pwm core
>>>>>>> instead. Currently there are two drivers known with this problem. I
>>>>>>> wouldn't be surprised if there were more.
>>>>>>
>>>>>> I've inspected all the drivers and didn't spot any beyond cros-ec and
>>>>>> i.MX that have this problem. There's also no good way to do this in the
>>>>>> core, because the core doesn't know whether or not the driver is capable
>>>>>> of returning the correct duty cycle on hardare readout. So the core
>>>>>> would have to rely on state->duty_cycle that is passed in, but then the
>>>>>> offending commit becomes useless because the whole point was to return
>>>>>> the state as written to hardware (rather than the software state which
>>>>>> was being returned before that patch).
>>>>>>
>>>>>>> If we want to move clients to not rely on .period and .duty_cycle for a
>>>>>>> disabled PWM (do we?) a single change in the core is also beneficial
>>>>>>> compared to fixing several lowlevel drivers.
>>>>>>
>>>>>> These are really two orthogonal problems. We don't currently consider
>>>>>> enabled = 0 to be equivalent to duty_cycle = 0 at an API level. I'm not
>>>>>> prepared to do that at this point in the release cycle either.
>>>>>>
>>>>>> What this here has shown is that we have at least two drivers that don't
>>>>>> behave the way they are supposed to according to the API and they break
>>>>>> consumers. If they break for pwm-backlight, it's possible that they will
>>>>>> break for other consumers as well. So the right thing to do is fix the
>>>>>> two drivers that are broken.
>>>>>>
>>>>>> After -rc1 we no longer experiment. Instead we clean up the messes we've
>>>>>> made. We can revisit the other points once mainline is fixed.
>>>>>
>>>>> Hi Thierry,
>>>>> I just tried your patch with v5.4-rc3 with this result:
>>>>>
>>>>> root at hydraco:~# dmesg | grep pwm_
>>>>> [ 1.772089] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
>>>>> [ 4.938759] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
>>>>> [ 4.947431] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
>>>>
>>>> Okay... this is interesting. If I understand correctly, that first line
>>>> here is where the initial hardware readout happens. The second one is
>>>> the first time when the backlight is configured, so it sets period and
>>>> polarity. But then for some reason when we read out after that to read
>>>> what state was written... we see that actually nothing was written at
>>>> all.
>>>>
>>>> And we can see why in pwm_imx27_apply(): If the PWM is not enabled, we
>>>> don't actually program any of the registers, so it's not a surprise that
>>>> things fall apart.
>>>>
>>>>> [ 4.956484] pwm_imx27_apply: period: 992970, duty: 992970, polarity: 0, enabled: 0
>>>>> [ 4.965473] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
>>>>> [ 4.974410] pwm_imx27_apply: period: 992970, duty: 0, polarity: 0, enabled: 1
>>>>> [ 4.988617] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 1
>>>>>
>>>>> Backlight is on with full brightness at this stage.
>>>>>
>>>>> root at hydraco:/sys/class/backlight/backlight# cat brightness
>>>>> 32
>>>>>
>>>>> root at hydraco:/sys/class/backlight/backlight# echo 32 > brightness
>>>>> [ 153.386391] pwm_imx27_apply: period: 992970, duty: 992970, polarity: 0, enabled: 1
>>>>> [ 153.398311] pwm_imx27_get_state: period: 992970, duty: 992970, polarity: 0, enabled: 1
>>>>>
>>>>> Backlight goes down.
>>>>>
>>>>> root at hydraco:/sys/class/backlight/backlight# echo 1 > brightness
>>>>> [ 168.506261] pwm_imx27_apply: period: 992970, duty: 15576, polarity: 0, enabled: 1
>>>>> [ 168.518064] pwm_imx27_get_state: period: 992970, duty: 15576, polarity: 0, enabled: 1
>>>>>
>>>>> Backlight goes up to almost full brightness.
>>>>>
>>>>> root at hydraco:/sys/class/backlight/backlight# echo 0 > brightness
>>>>> [ 177.496265] pwm_imx27_apply: period: 992970, duty: 0, polarity: 0, enabled: 0
>>>>> [ 177.507602] pwm_imx27_get_state: period: 496485, duty: 7788, polarity: 0, enabled: 0
>>>>>
>>>>> Backlight goes up to full brightness.
>>>>>
>>>>> So your patch does not solve my issue.
>>>>>
>>>>> The main problem I see is incorrect polarity setting. In my DT
>>>>> the pwm-backlight consumer requests PWM_POLARITY_INVERTED and
>>>>> period 500000ns. Though after reset the PWM HW registers are
>>>>> configured to normal polarity. This initial setting is read out
>>>>> and used by the consumer instead of the DT configuration.
>>>>
>>>> So the problem with the i.MX driver is that it doesn't actually write
>>>> the full state to the hardware and therefore the patch that caused these
>>>> things to break reads back an incomplete state. So we've basically got
>>>> two options: 1) make sure the hardware state is fully written or 2) make
>>>> sure that we return the cached state.
>>>>
>>>> I think 2) doesn't really make sense because it is conflicts with the
>>>> purpose of the ->get_state() callback. The only time where we should be
>>>> returning cached data is if the hardware registers don't contain the
>>>> information (as in the case of the cros-ec driver) or if we can't access
>>>> it for other reasons (such as in the case of i.MX's duty cycle).
>>>>
>>>> Does the attached patch help with your issue? The idea is to always
>>>> write the full state to the hardware, even if period and duty cycle are
>>>> unused when the PWM is disabled. That's really the kind of contract that
>>>> we have added with the offending patch in the core.
>>>>
>>>> It looks like all other drivers handle this more or less correctly, so
>>>> if we only need to fix up cros-ec and i.MX this seems like a realistic
>>>> way to fix things up. If other drivers are problematic in this regard,
>>>> we should probably revert and then fix the drivers before we can apply
>>>> that patch again.
>>>
>>> This patch combined with your previous patch appears to have worked.
>>> If you end up sending a patch series to fix this, go ahead and add
>>>
>>> Tested-by: Adam Ford <aford173 at gmail.com> #imx6q-logicpd
>>
>> Excellent! Thanks for testing this. I'll wait until tomorrow to see if
>> there's some feedback from Enric for the cros-ec change. I'll send out
>> the total of three patches again in the hopes that those are really
>> the only two cases that are broken.
>
> When you do, can you mark it with the Fixes note? I am hoping the
> maintainers can hopefully incorporate this into 5.4 since it fixes a
> regression.
Hi Thierry,
I can confirm that the combination of your two patches:
- ("pwm: imx27: Unconditionally write state to hardware")
- ("pwm: imx27: Cache duty cycle register value")
works OK and solve my problem as well.
root at hydraco:~# dmesg | grep pwm_
[ 1.695306] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
[ 5.387271] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 5.396433] pwm_imx27_get_state: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 5.405500] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 0
[ 5.418802] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 0
[ 5.428208] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 1
[ 5.442633] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 1
Backlight is on from power up to userspace.
root at hydraco:~# cd /sys/class/backlight/backlight/
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# cat brightness
32
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 32 > brightness
Nothing happens.
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 1 > brightness
[ 513.629043] pwm_imx27_apply: period: 500000, duty: 7843, polarity: 1, enabled: 1
[ 513.639899] pwm_imx27_get_state: period: 500000, duty: 7833, polarity: 1, enabled: 1
Backlight goes to low brightness.
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 0 > brightness
[ 519.677088] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 519.687733] pwm_imx27_get_state: period: 500000, duty: 0, polarity: 1, enabled: 0
Backlight goes to max brightness, unresolved i.MX6 limitation.
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 32 > brightness
[ 923.921292] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 0
[ 923.933331] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 0
[ 923.944546] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 1
[ 923.963931] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 1
Backlight remains at max brightness, OK.
If I apply the patch from Uwe ("backlight: pwm_bl: configure pwm only once
per backlight toggle") on top of that, it still works and I do not see
any change in the behavior.
root at hydraco:~# dmesg | grep pwm_
[ 1.687461] pwm_imx27_get_state: period: 992970, duty: 0, polarity: 0, enabled: 0
[ 4.875087] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 4.884796] pwm_imx27_get_state: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 4.893922] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 1
[ 4.908473] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 1
root at hydraco:~# cd /sys/class/backlight/backlight/
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# cat brightness
32
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 32 > brightness
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 1 > brightness
[ 110.775650] pwm_imx27_apply: period: 500000, duty: 7843, polarity: 1, enabled: 1
[ 110.786512] pwm_imx27_get_state: period: 500000, duty: 7833, polarity: 1, enabled: 1
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 0 > brightness
[ 128.224036] pwm_imx27_apply: period: 500000, duty: 0, polarity: 1, enabled: 0
[ 128.234675] pwm_imx27_get_state: period: 500000, duty: 0, polarity: 1, enabled: 0
root at hydraco:/sys/devices/soc0/backlight/backlight/backlight# echo 32 > brightness
[ 138.208072] pwm_imx27_apply: period: 500000, duty: 500000, polarity: 1, enabled: 1
[ 138.220271] pwm_imx27_get_state: period: 500000, duty: 500000, polarity: 1, enabled: 1
The only difference is here when the state is changed from enabled=0
to enabled=1. The apply/get_state combo is called only once.
So this looks good to me.
Tested-by: Michal Vokáč <michal.vokac at ysoft.com>
Thank you all for the very prompt reaction!
Michal
>>>> --- >8 ---
>>>> From 7040f0038e04a1caa6dda5b6f675a9fdee0271f4 Mon Sep 17 00:00:00 2001
>>>> From: Thierry Reding <thierry.reding at gmail.com>
>>>> Date: Thu, 17 Oct 2019 17:11:41 +0200
>>>> Subject: [PATCH] pwm: imx27: Unconditionally write state to hardware
>>>>
>>>> The i.MX driver currently uses a shortcut and doesn't write all of the
>>>> state through to the hardware when the PWM is disabled. This causes an
>>>> inconsistent state to be read back by consumers with the result of them
>>>> malfunctioning.
>>>>
>>>> Fix this by always writing the full state through to the hardware
>>>> registers so that the correct state can always be read back.
>>>>
>>>> Signed-off-by: Thierry Reding <thierry.reding at gmail.com>
>>>> ---
>>>> drivers/pwm/pwm-imx27.c | 120 ++++++++++++++++++++--------------------
>>>> 1 file changed, 59 insertions(+), 61 deletions(-)
>>>>
>>>> diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
>>>> index 4113d5cd4c62..59d8b1289808 100644
>>>> --- a/drivers/pwm/pwm-imx27.c
>>>> +++ b/drivers/pwm/pwm-imx27.c
>>>> @@ -230,70 +230,68 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>>>>
>>>> pwm_get_state(pwm, &cstate);
>>>>
>>>> - if (state->enabled) {
>>>> - c = clk_get_rate(imx->clk_per);
>>>> - c *= state->period;
>>>> -
>>>> - do_div(c, 1000000000);
>>>> - period_cycles = c;
>>>> -
>>>> - prescale = period_cycles / 0x10000 + 1;
>>>> -
>>>> - period_cycles /= prescale;
>>>> - c = (unsigned long long)period_cycles * state->duty_cycle;
>>>> - do_div(c, state->period);
>>>> - duty_cycles = c;
>>>> -
>>>> - /*
>>>> - * according to imx pwm RM, the real period value should be
>>>> - * PERIOD value in PWMPR plus 2.
>>>> - */
>>>> - if (period_cycles > 2)
>>>> - period_cycles -= 2;
>>>> - else
>>>> - period_cycles = 0;
>>>> -
>>>> - /*
>>>> - * Wait for a free FIFO slot if the PWM is already enabled, and
>>>> - * flush the FIFO if the PWM was disabled and is about to be
>>>> - * enabled.
>>>> - */
>>>> - if (cstate.enabled) {
>>>> - pwm_imx27_wait_fifo_slot(chip, pwm);
>>>> - } else {
>>>> - ret = pwm_imx27_clk_prepare_enable(chip);
>>>> - if (ret)
>>>> - return ret;
>>>> -
>>>> - pwm_imx27_sw_reset(chip);
>>>> - }
>>>> -
>>>> - writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
>>>> - writel(period_cycles, imx->mmio_base + MX3_PWMPR);
>>>> -
>>>> - /*
>>>> - * Store the duty cycle for future reference in cases where
>>>> - * the MX3_PWMSAR register can't be read (i.e. when the PWM
>>>> - * is disabled).
>>>> - */
>>>> - imx->duty_cycle = duty_cycles;
>>>> -
>>>> - cr = MX3_PWMCR_PRESCALER_SET(prescale) |
>>>> - MX3_PWMCR_STOPEN | MX3_PWMCR_DOZEN | MX3_PWMCR_WAITEN |
>>>> - FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) |
>>>> - MX3_PWMCR_DBGEN | MX3_PWMCR_EN;
>>>> -
>>>> - if (state->polarity == PWM_POLARITY_INVERSED)
>>>> - cr |= FIELD_PREP(MX3_PWMCR_POUTC,
>>>> - MX3_PWMCR_POUTC_INVERTED);
>>>> -
>>>> - writel(cr, imx->mmio_base + MX3_PWMCR);
>>>> - } else if (cstate.enabled) {
>>>> - writel(0, imx->mmio_base + MX3_PWMCR);
>>>> + c = clk_get_rate(imx->clk_per);
>>>> + c *= state->period;
>>>>
>>>> - pwm_imx27_clk_disable_unprepare(chip);
>>>> + do_div(c, 1000000000);
>>>> + period_cycles = c;
>>>> +
>>>> + prescale = period_cycles / 0x10000 + 1;
>>>> +
>>>> + period_cycles /= prescale;
>>>> + c = (unsigned long long)period_cycles * state->duty_cycle;
>>>> + do_div(c, state->period);
>>>> + duty_cycles = c;
>>>> +
>>>> + /*
>>>> + * according to imx pwm RM, the real period value should be PERIOD
>>>> + * value in PWMPR plus 2.
>>>> + */
>>>> + if (period_cycles > 2)
>>>> + period_cycles -= 2;
>>>> + else
>>>> + period_cycles = 0;
>>>> +
>>>> + /*
>>>> + * Wait for a free FIFO slot if the PWM is already enabled, and flush
>>>> + * the FIFO if the PWM was disabled and is about to be enabled.
>>>> + */
>>>> + if (cstate.enabled) {
>>>> + pwm_imx27_wait_fifo_slot(chip, pwm);
>>>> + } else {
>>>> + ret = pwm_imx27_clk_prepare_enable(chip);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + pwm_imx27_sw_reset(chip);
>>>> }
>>>>
>>>> + writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
>>>> + writel(period_cycles, imx->mmio_base + MX3_PWMPR);
>>>> +
>>>> + /*
>>>> + * Store the duty cycle for future reference in cases where the
>>>> + * MX3_PWMSAR register can't be read (i.e. when the PWM is disabled).
>>>> + */
>>>> + imx->duty_cycle = duty_cycles;
>>>> +
>>>> + cr = MX3_PWMCR_PRESCALER_SET(prescale) |
>>>> + MX3_PWMCR_STOPEN | MX3_PWMCR_DOZEN | MX3_PWMCR_WAITEN |
>>>> + FIELD_PREP(MX3_PWMCR_CLKSRC, MX3_PWMCR_CLKSRC_IPG_HIGH) |
>>>> + MX3_PWMCR_DBGEN;
>>>> +
>>>> + if (state->polarity == PWM_POLARITY_INVERSED)
>>>> + cr |= FIELD_PREP(MX3_PWMCR_POUTC,
>>>> + MX3_PWMCR_POUTC_INVERTED);
>>>> +
>>>> + if (state->enabled)
>>>> + cr |= MX3_PWMCR_EN;
>>>> +
>>>> + writel(cr, imx->mmio_base + MX3_PWMCR);
>>>> +
>>>> + if (!state->enabled && cstate.enabled)
>>>> + pwm_imx27_clk_disable_unprepare(chip);
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> --
>>>> 2.23.0
>>>>
More information about the dri-devel
mailing list