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.

Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Acked-by: Thierry Reding <thierry.red...@gmail.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v3:
- Use NSEC_PER_USEC instead of adding a new (non-sensical) NSEC_PER_MHZ define
---
 drivers/pwm/pwm-crc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index 272eeb071147..c056eb9b858c 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -21,8 +21,8 @@
 
 #define PWM_MAX_LEVEL          0xFF
 
-#define PWM_BASE_CLK           6000000  /* 6 MHz */
-#define PWM_MAX_PERIOD_NS      21333    /* 46.875KHz */
+#define PWM_BASE_CLK_MHZ       6       /* 6 MHz */
+#define PWM_MAX_PERIOD_NS      5461333 /* 183 Hz */
 
 /**
  * struct crystalcove_pwm - Crystal Cove PWM controller
@@ -72,7 +72,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct 
pwm_device *pwm,
 
                /* changing the clk divisor, need to disable fisrt */
                crc_pwm_disable(c, pwm);
-               clk_div = PWM_BASE_CLK * period_ns / NSEC_PER_SEC;
+               clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_USEC);
 
                regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
                                        clk_div | PWM_OUTPUT_ENABLE);
-- 
2.28.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to