On Sun, Jun 07, 2020 at 08:18:31PM +0200, Hans de Goede wrote:
While looking into adding atomic-pwm support to the pwm-crc driver I noticed something odd, there is a PWM_BASE_CLK define of 6 MHz and there is a clock-divider which divides this with a value between 1-128, and there are 256 duty-cycle steps.
The pwm-crc code before this commit assumed that a clock-divider setting of 1 means that the PWM output is running at 6 MHZ, if that is true, where do these 256 duty-cycle steps come from?
This would require an internal frequency of 256 * 6 MHz = 1.5 GHz, that seems unlikely for a PMIC which is using a silicon process optimized for power-switching transistors. It is way more likely that there is an 8 bit counter for the duty cycle which acts as an extra fixed divider wrt the PWM output frequency.
The main user of the pwm-crc driver is the i915 GPU driver which uses it for backlight control. Lets compare the PWM register values set by the video-BIOS (the GOP), assuming the extra fixed divider is present versus the PWM frequency specified in the Video-BIOS-Tables:
Device: PWM Hz set by BIOS PWM Hz specified in VBT Asus T100TA 200 200 Asus T100HA 200 200 Lenovo Miix 2 8 23437 20000 Toshiba WT8-A 23437 20000
So as we can see if we assume the extra division by 256 then the register values set by the GOP are an exact match for the VBT values, where as otherwise the values would be of by a factor of 256.
This commit fixes the period / duty_cycle calculations to take the extra division by 256 into account.
...
+#define NSEC_PER_MHZ 1000
This is against physics. What this cryptic name means actually? Existing NSEC_PER_USEC ?