Hello,
there was some off-list discussion about this problem. I summarize the
technical details and findings here.
On Thu, Aug 18, 2022 at 11:34:03PM +0300, Dmitry Osipenko wrote:
> On 8/18/22 10:54, Uwe Kleine-König wrote:
> > On Mon, Aug 15, 2022 at 09:09:35AM +0200, Uwe Kleine-König wrote:
> >> On Mon, Aug 15, 2022 at 03:28:25AM +0300, Dmitry Osipenko wrote:
> >>> This patch broke backlight on Asus Transformer tablets, they are now
> >>> getting this -EINVAL. The root of the problem is under investigation.
> >>
> >> This means that you requested a period that is smaller than the minimal
> >> period the hardware can implement.
> >>
> >> What is the clk rate of the PWM clk (i.e. pc->clk_rate?). Looking at
> >> arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi I guess period is
> >> 4000000. That in turn would mean that
I got some traces, and while I don't understand the whole picture yet,
they suggest the period is 1000000 ns only.
> >>
> >> mul_u64_u64_div_u64(pc->clk_rate, period_ns, (u64)NSEC_PER_SEC << PWM_DUTY_WIDTH)
> >>
> >> returned 0 which (with the assumption period_ns = 4000000) would imply
> >> the clk rate is less than 64000.
As the clk-rate is only 32768 Hz we get (with period_ns = 1000000)
32768 * 1000000 / (1000000000 << 8) = 0.128
which is indeed rounded down to 0 and then runs into the error path
returning -EINVAL. Before my change (that now broke the backlight
configuration) configuration continued and then ended with actually
configuring period = 7812500 ns which is off by nearly a factor of 8.
I didn't find a device tree for an Asus Transformer tablet bases on a
tegra124 in the kernel source, but the options are:
- Revert commit 8c193f4714df ("pwm: tegra: Optimize period calculation").
I don't like this. IMHO this commit is an improvement and the problem
is that the consumer requests a too small period. For a backlight
this might be ok to result in a much bigger period, for other
usecases it isn't and so I like refusing period = 1000000.
- We could just drop the "else / return -EINVAL".
This is inconsistent as then (again) some periods are rounded up
(with the given clk rate that would be 5334 <= period < 7812500)
while others (period < 5334) yield -EINVAL.
- Increase the period that the backlight is using to at least 7812500.
This is done (I guess) by replacing 1000000 by 7812500 (or more) in
the backlight's PWM phandle.
- Make the PWM clk faster.
Looking quickly through the tegra clk driver, the parent of the PWM
clk could be changed from clk_32k to pll_p or pll_c. This should be
doable in the dts using something like:
assigned-clocks = <&tegra_car TEGRA124_CLK_PWM>;
assigned-clock-parents = <&tegra_car TEGRA124_CLK_PLL_P>;
in the pwm node. (Note this includes much guesswork, I don't know the
PPL's clk-rate, so this might break in other ways. Another option is
using PLL_C.)
Probably the latter is the nicest option. Is it possible to find out the
setting when the machine is running the original vendor OS?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
