[PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

This helps to get 100% intensity closer to "always on".

It compensates for an effect of dmtimer which at 100% still emits short
"off" impulses and the startup-time of the DC/DC converter makes
backlight intensity not reach full scale. The lower the PWM frequency
is, the smaller is this effect.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap3-gta04.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index c09a057..94f12dd 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -102,7 +102,7 @@
 
 	backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm11 0 2000000 0>;
+		pwms = <&pwm11 0 12000000 0>;
 		pwm-names = "backlight";
 		brightness-levels = <0 11 20 30 40 50 60 70 80 90 100>;
 		default-brightness-level = <9>;	/* => 90 */
-- 
2.7.3

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Matthijs van Duin]

On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
> This helps to get 100% intensity closer to "always on".
> 
> It compensates for an effect of dmtimer which at 100% still emits short
> "off" impulses and the startup-time of the DC/DC converter makes
> backlight intensity not reach full scale. The lower the PWM frequency
> is, the smaller is this effect.

Sounds to me like you're working around something that should be fixed
in the pwm-omap-dmtimer driver instead?

Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
supports fully off to fully on, I do seem to be handling both endpoints
in a special way.  A rough conversion of my code into C:

// period in timer cycles
void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
{
	assert( period >= 2 );
	timer->if_ctrl = 2;  // reset timer, configure as non-posted
	timer->reload = -period;
	timer->trigger = 0;
	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
}

// value in timer cycles, 0 <= value <= period
void pwm_set( volatile struct dmtimer *timer, u32 value )
{
	if( value == 0 ) {
		timer->config &= ~0x800;  // disable pwm
		return;
	}
	u32 period = -timer->reload;
	if( value >= period )
		timer->match = 0;
	else
		timer->match = value - period - 1;
	timer->config |= 0x800;  // enable pwm
}

At the time I used a scope to check the exact behaviour of dmtimer pwm
on a dm814x.  My notes mention (when pwm enabled):
	match < reload	output on continuous
	match == reload	output on 1 cycle, off period-1 cycles
	match == -2	output on period-1 cycles, off 1 cycle
	match == -1	output freezes

Hope this helps

Matthijs

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

Hi,

> Am 10.09.2016 um 05:17 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
> 
> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
>> This helps to get 100% intensity closer to "always on".
>> 
>> It compensates for an effect of dmtimer which at 100% still emits short
>> "off" impulses and the startup-time of the DC/DC converter makes
>> backlight intensity not reach full scale. The lower the PWM frequency
>> is, the smaller is this effect.
> 
> Sounds to me like you're working around something that should be fixed

Yes and no.

Reducing the PWM frequency is good by itself since it should not be unnecessarily
fast and helps to make the PWM to "average current" translation more linear.

The non-linear effect is that the PWM controlled DC/DC converter reacts almost
immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
on a 0->1 transition. So if you run PWM @ 500Hz and 100% there is 1ms 1 and 1 ms 0.
But this translates to 1.5 ms no power and 0.5ms power which is 50% of the intended
current.

This gives some "reduction" factor to all PWM duty cycle values, but the 100%
case is the most noticeable one.

If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
very significant change if we switch to 99%, depending on PWM frequency.

This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
reasonable (although still a little arbitrary) than the current value. (BTW: for
the Pyra we already use 83Hz).

> in the pwm-omap-dmtimer driver instead?

Yes, it probably should be fixed as well but it does not completely solve
the backlight control issue due to the DC/DC converter's behaviour.

> 
> Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
> supports fully off to fully on, I do seem to be handling both endpoints
> in a special way.  A rough conversion of my code into C:
> 
> // period in timer cycles
> void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
> {
> 	assert( period >= 2 );
> 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
> 	timer->reload = -period;
> 	timer->trigger = 0;
> 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
> }
> 
> // value in timer cycles, 0 <= value <= period
> void pwm_set( volatile struct dmtimer *timer, u32 value )
> {
> 	if( value == 0 ) {
> 		timer->config &= ~0x800;  // disable pwm
> 		return;
> 	}
> 	u32 period = -timer->reload;
> 	if( value >= period )
> 		timer->match = 0;
> 	else
> 		timer->match = value - period - 1;
> 	timer->config |= 0x800;  // enable pwm
> }
> 
> At the time I used a scope to check the exact behaviour of dmtimer pwm
> on a dm814x.  My notes mention (when pwm enabled):
> 	match < reload	output on continuous
> 	match == reload	output on 1 cycle, off period-1 cycles
> 	match == -2	output on period-1 cycles, off 1 cycle
> 	match == -1	output freezes
> 
> Hope this helps
> 
> Matthijs

BR,
Nikolaus

Re: [Letux-kernel] [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

> Am 10.09.2016 um 09:08 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi,
> 
>> Am 10.09.2016 um 05:17 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
>> 
>> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
>>> This helps to get 100% intensity closer to "always on".
>>> 
>>> It compensates for an effect of dmtimer which at 100% still emits short
>>> "off" impulses and the startup-time of the DC/DC converter makes
>>> backlight intensity not reach full scale. The lower the PWM frequency
>>> is, the smaller is this effect.
>> 
>> Sounds to me like you're working around something that should be fixed
> 
> Yes and no.
> 
> Reducing the PWM frequency is good by itself since it should not be unnecessarily
> fast and helps to make the PWM to "average current" translation more linear.
> 
> The non-linear effect is that the PWM controlled DC/DC converter reacts almost
> immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
> on a 0->1 transition. So if you run PWM @ 500Hz and 100% there is 1ms 1 and 1 ms 0.

was too early in the morning: should be PWM @ 500Hz and 50%.

> But this translates to 1.5 ms no power and 0.5ms power which is 50% of the intended
> current.

At PWM at 100% with the current PWM driver we still get a DC/DC control of 1.95ms 1
and 0.05ms 0 which translates into 0.55ms no power and 1.45 ms power which is only 75%
of the desired maximum.

> 
> This gives some "reduction" factor to all PWM duty cycle values, but the 100%
> case is the most noticeable one.
> 
> If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
> very significant change if we switch to 99%, depending on PWM frequency.
> 
> This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
> reasonable (although still a little arbitrary) than the current value. (BTW: for
> the Pyra we already use 83Hz).
> 
>> in the pwm-omap-dmtimer driver instead?
> 
> Yes, it probably should be fixed as well but it does not completely solve
> the backlight control issue due to the DC/DC converter's behaviour.
> 
>> 
>> Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
>> supports fully off to fully on, I do seem to be handling both endpoints
>> in a special way.  A rough conversion of my code into C:
>> 
>> // period in timer cycles
>> void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
>> {
>> 	assert( period >= 2 );
>> 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
>> 	timer->reload = -period;
>> 	timer->trigger = 0;
>> 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
>> }
>> 
>> // value in timer cycles, 0 <= value <= period
>> void pwm_set( volatile struct dmtimer *timer, u32 value )
>> {
>> 	if( value == 0 ) {
>> 		timer->config &= ~0x800;  // disable pwm
>> 		return;
>> 	}
>> 	u32 period = -timer->reload;
>> 	if( value >= period )
>> 		timer->match = 0;
>> 	else
>> 		timer->match = value - period - 1;
>> 	timer->config |= 0x800;  // enable pwm
>> }
>> 
>> At the time I used a scope to check the exact behaviour of dmtimer pwm
>> on a dm814x.  My notes mention (when pwm enabled):
>> 	match < reload	output on continuous
>> 	match == reload	output on 1 cycle, off period-1 cycles
>> 	match == -2	output on period-1 cycles, off 1 cycle
>> 	match == -1	output freezes
>> 
>> Hope this helps
>> 
>> Matthijs
> 
> BR,
> Nikolaus
> 
> _______________________________________________
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> Letux-kernel@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

Re: [Letux-kernel] [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

> Am 10.09.2016 um 09:08 schrieb H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>:
> 
> Hi,
> 
>> Am 10.09.2016 um 05:17 schrieb Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> 
>> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
>>> This helps to get 100% intensity closer to "always on".
>>> 
>>> It compensates for an effect of dmtimer which at 100% still emits short
>>> "off" impulses and the startup-time of the DC/DC converter makes
>>> backlight intensity not reach full scale. The lower the PWM frequency
>>> is, the smaller is this effect.
>> 
>> Sounds to me like you're working around something that should be fixed
> 
> Yes and no.
> 
> Reducing the PWM frequency is good by itself since it should not be unnecessarily
> fast and helps to make the PWM to "average current" translation more linear.
> 
> The non-linear effect is that the PWM controlled DC/DC converter reacts almost
> immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
> on a 0->1 transition. So if you run PWM @ 500Hz and 100% there is 1ms 1 and 1 ms 0.

was too early in the morning: should be PWM @ 500Hz and 50%.

> But this translates to 1.5 ms no power and 0.5ms power which is 50% of the intended
> current.

At PWM at 100% with the current PWM driver we still get a DC/DC control of 1.95ms 1
and 0.05ms 0 which translates into 0.55ms no power and 1.45 ms power which is only 75%
of the desired maximum.

> 
> This gives some "reduction" factor to all PWM duty cycle values, but the 100%
> case is the most noticeable one.
> 
> If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
> very significant change if we switch to 99%, depending on PWM frequency.
> 
> This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
> reasonable (although still a little arbitrary) than the current value. (BTW: for
> the Pyra we already use 83Hz).
> 
>> in the pwm-omap-dmtimer driver instead?
> 
> Yes, it probably should be fixed as well but it does not completely solve
> the backlight control issue due to the DC/DC converter's behaviour.
> 
>> 
>> Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
>> supports fully off to fully on, I do seem to be handling both endpoints
>> in a special way.  A rough conversion of my code into C:
>> 
>> // period in timer cycles
>> void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
>> {
>> 	assert( period >= 2 );
>> 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
>> 	timer->reload = -period;
>> 	timer->trigger = 0;
>> 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
>> }
>> 
>> // value in timer cycles, 0 <= value <= period
>> void pwm_set( volatile struct dmtimer *timer, u32 value )
>> {
>> 	if( value == 0 ) {
>> 		timer->config &= ~0x800;  // disable pwm
>> 		return;
>> 	}
>> 	u32 period = -timer->reload;
>> 	if( value >= period )
>> 		timer->match = 0;
>> 	else
>> 		timer->match = value - period - 1;
>> 	timer->config |= 0x800;  // enable pwm
>> }
>> 
>> At the time I used a scope to check the exact behaviour of dmtimer pwm
>> on a dm814x.  My notes mention (when pwm enabled):
>> 	match < reload	output on continuous
>> 	match == reload	output on 1 cycle, off period-1 cycles
>> 	match == -2	output on period-1 cycles, off 1 cycle
>> 	match == -1	output freezes
>> 
>> Hope this helps
>> 
>> Matthijs
> 
> BR,
> Nikolaus
> 
> _______________________________________________
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> Letux-kernel-S0jZdbWzriLCfDggNXIi3w@public.gmane.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Matthijs van Duin]

On 10 September 2016 at 09:08, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Reducing the PWM frequency is good by itself since it should not be unnecessarily
> fast and helps to make the PWM to "average current" translation more linear.
>
> The non-linear effect is that the PWM controlled DC/DC converter reacts almost
> immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
> on a 0->1 transition.

DT already allows for compensation of many non-linearities by
specifying the duty cycle of each brightness increment.  Though, as
you observed, there's one limitation it cannot fix here:

> If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
> very significant change if we switch to 99%, depending on PWM frequency.

Specifically the next-to-brightest step (assuming 0.5ms off-time) would be:
75%  @ 500 Hz
90%  @ 200 Hz
95%  @ 100 Hz
96%  @  83 Hz

Note that perceptually the distance to 100% might be smaller due to
non-linear response of the eye. That's my experience with pwm
controlled leds anyway, which may or may not apply to backlights
(though with my laptop's backlight I never really have use for the
distinct steps at the brightest end while those at the darkest end
seem disproportionally large).

> This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
> reasonable (although still a little arbitrary) than the current value.

While 500Hz is perhaps a bit high, 83Hz actually seems very low to me.
Searching a bit around yielded 175 Hz as common frequency for CCFL
backlights and higher for LED backlights (source:
http://www.tftcentral.co.uk/articles/pulse_width_modulation.htm).

(I may be reacting a bit twitchy here due to having encountered dimmed
LED lighting that was flickering obnoxiously for me while noone else
noticed this.)

Matthijs

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Matthijs van Duin]

On 10 September 2016 at 09:08, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
> Reducing the PWM frequency is good by itself since it should not be unnecessarily
> fast and helps to make the PWM to "average current" translation more linear.
>
> The non-linear effect is that the PWM controlled DC/DC converter reacts almost
> immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
> on a 0->1 transition.

DT already allows for compensation of many non-linearities by
specifying the duty cycle of each brightness increment.  Though, as
you observed, there's one limitation it cannot fix here:

> If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
> very significant change if we switch to 99%, depending on PWM frequency.

Specifically the next-to-brightest step (assuming 0.5ms off-time) would be:
75%  @ 500 Hz
90%  @ 200 Hz
95%  @ 100 Hz
96%  @  83 Hz

Note that perceptually the distance to 100% might be smaller due to
non-linear response of the eye. That's my experience with pwm
controlled leds anyway, which may or may not apply to backlights
(though with my laptop's backlight I never really have use for the
distinct steps at the brightest end while those at the darkest end
seem disproportionally large).

> This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
> reasonable (although still a little arbitrary) than the current value.

While 500Hz is perhaps a bit high, 83Hz actually seems very low to me.
Searching a bit around yielded 175 Hz as common frequency for CCFL
backlights and higher for LED backlights (source:
http://www.tftcentral.co.uk/articles/pulse_width_modulation.htm).

(I may be reacting a bit twitchy here due to having encountered dimmed
LED lighting that was flickering obnoxiously for me while noone else
noticed this.)

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

Hi,

> Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
> 
> On 10 September 2016 at 09:08, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Reducing the PWM frequency is good by itself since it should not be unnecessarily
>> fast and helps to make the PWM to "average current" translation more linear.
>> 
>> The non-linear effect is that the PWM controlled DC/DC converter reacts almost
>> immediately to a 1->0 control transition but needs some time (ca. 0.5ms) to recover
>> on a 0->1 transition.
> 
> DT already allows for compensation of many non-linearities by
> specifying the duty cycle of each brightness increment.  Though, as
> you observed, there's one limitation it cannot fix here:
> 
>> If we just fix the PWM generator to output a steady 1 signal at 100%, we have a
>> very significant change if we switch to 99%, depending on PWM frequency.
> 
> Specifically the next-to-brightest step (assuming 0.5ms off-time) would be:
> 75%  @ 500 Hz
> 90%  @ 200 Hz
> 95%  @ 100 Hz
> 96%  @  83 Hz

Yes.

> 
> Note that perceptually the distance to 100% might be smaller due to
> non-linear response of the eye. That's my experience with pwm
> controlled leds anyway, which may or may not apply to backlights

basically it does. Eye is basically logarithmic - but has several auto-exposure
and auto-iris mechanisms... So perceived brightness is a very complex topic.
It might even depend on the color and contrast of the image presented. This
is something we can ever fix by DT...

> (though with my laptop's backlight I never really have use for the
> distinct steps at the brightest end while those at the darkest end
> seem disproportionally large).
> 
>> This effect becomes smaller if the PWM frequency is reduced and 83Hz seems more
>> reasonable (although still a little arbitrary) than the current value.
> 
> While 500Hz is perhaps a bit high, 83Hz actually seems very low to me.

Why? The eye can't even see flicker @ 50 Hz.

And, there is a capacitor that averages the voltage applied, hence it is
low pass filtered. But the capacitor can't compensate for the startup delay
of the DC/DC converter.

And, I have tested that on the device targeted by this DTS... No visible
issue (except that maximum brightness decreases if too high).

> Searching a bit around yielded 175 Hz as common frequency for CCFL
> backlights and higher for LED backlights (source:
> http://www.tftcentral.co.uk/articles/pulse_width_modulation.htm).

> 
> (I may be reacting a bit twitchy here due to having encountered dimmed
> LED lighting that was flickering obnoxiously for me while noone else
> noticed this.)
> 
> Matthijs

But with the patch submitted, I just want to give the dts of a single
device I have even designed a more reasonable value than in current
linux/master and don't really want to make it a fundamental discussion...

BR,
Nikolaus

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Neil Armstrong]

Le 10/09/2016 05:17, Matthijs van Duin a écrit :
> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
>> This helps to get 100% intensity closer to "always on".
>>
>> It compensates for an effect of dmtimer which at 100% still emits short
>> "off" impulses and the startup-time of the DC/DC converter makes
>> backlight intensity not reach full scale. The lower the PWM frequency
>> is, the smaller is this effect.
> 
> Sounds to me like you're working around something that should be fixed
> in the pwm-omap-dmtimer driver instead?
> 
> Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
> supports fully off to fully on, I do seem to be handling both endpoints
> in a special way.  A rough conversion of my code into C:
> 
> // period in timer cycles
> void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
> {
> 	assert( period >= 2 );
> 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
> 	timer->reload = -period;
> 	timer->trigger = 0;
> 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
> }
> 
> // value in timer cycles, 0 <= value <= period
> void pwm_set( volatile struct dmtimer *timer, u32 value )
> {
> 	if( value == 0 ) {
> 		timer->config &= ~0x800;  // disable pwm
> 		return;
> 	}
> 	u32 period = -timer->reload;
> 	if( value >= period )
> 		timer->match = 0;
> 	else
> 		timer->match = value - period - 1;
> 	timer->config |= 0x800;  // enable pwm
> }
> 
> At the time I used a scope to check the exact behaviour of dmtimer pwm
> on a dm814x.  My notes mention (when pwm enabled):
> 	match < reload	output on continuous
> 	match == reload	output on 1 cycle, off period-1 cycles
> 	match == -2	output on period-1 cycles, off 1 cycle
> 	match == -1	output freezes
> 
> Hope this helps

Hi,

I think these corner cases should definitely be handled in the dmtimer driver.

I'll try to post a fix to handle these, thanks for the original code dump.

> 
> Matthijs
> 

Neil

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Neil Armstrong]

Le 10/09/2016 05:17, Matthijs van Duin a écrit :
> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:
>> This helps to get 100% intensity closer to "always on".
>>
>> It compensates for an effect of dmtimer which at 100% still emits short
>> "off" impulses and the startup-time of the DC/DC converter makes
>> backlight intensity not reach full scale. The lower the PWM frequency
>> is, the smaller is this effect.
> 
> Sounds to me like you're working around something that should be fixed
> in the pwm-omap-dmtimer driver instead?
> 
> Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
> supports fully off to fully on, I do seem to be handling both endpoints
> in a special way.  A rough conversion of my code into C:
> 
> // period in timer cycles
> void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
> {
> 	assert( period >= 2 );
> 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
> 	timer->reload = -period;
> 	timer->trigger = 0;
> 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
> }
> 
> // value in timer cycles, 0 <= value <= period
> void pwm_set( volatile struct dmtimer *timer, u32 value )
> {
> 	if( value == 0 ) {
> 		timer->config &= ~0x800;  // disable pwm
> 		return;
> 	}
> 	u32 period = -timer->reload;
> 	if( value >= period )
> 		timer->match = 0;
> 	else
> 		timer->match = value - period - 1;
> 	timer->config |= 0x800;  // enable pwm
> }
> 
> At the time I used a scope to check the exact behaviour of dmtimer pwm
> on a dm814x.  My notes mention (when pwm enabled):
> 	match < reload	output on continuous
> 	match == reload	output on 1 cycle, off period-1 cycles
> 	match == -2	output on period-1 cycles, off 1 cycle
> 	match == -1	output freezes
> 
> Hope this helps

Hi,

I think these corner cases should definitely be handled in the dmtimer driver.

I'll try to post a fix to handle these, thanks for the original code dump.

> 
> Matthijs
> 

Neil
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[David Rivshin]

On Sat, 10 Sep 2016 15:48:28 +0200
Neil Armstrong <narmstrong@baylibre.com> wrote:

> Le 10/09/2016 05:17, Matthijs van Duin a écrit :
> > On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:  
> >> This helps to get 100% intensity closer to "always on".
> >>
> >> It compensates for an effect of dmtimer which at 100% still emits short
> >> "off" impulses and the startup-time of the DC/DC converter makes
> >> backlight intensity not reach full scale. The lower the PWM frequency
> >> is, the smaller is this effect.  
> > 
> > Sounds to me like you're working around something that should be fixed
> > in the pwm-omap-dmtimer driver instead?
> > 
> > Looking at the (baremetal) dmtimer pwm code I wrote ages ago, which
> > supports fully off to fully on, I do seem to be handling both endpoints
> > in a special way.  A rough conversion of my code into C:
> > 
> > // period in timer cycles
> > void pwm_init( volatile struct dmtimer *timer, u32 period, bool invert )
> > {
> > 	assert( period >= 2 );
> > 	timer->if_ctrl = 2;  // reset timer, configure as non-posted
> > 	timer->reload = -period;
> > 	timer->trigger = 0;
> > 	timer->config = 0x1043 | invert << 7;  // pwm initially disabled
> > }
> > 
> > // value in timer cycles, 0 <= value <= period
> > void pwm_set( volatile struct dmtimer *timer, u32 value )
> > {
> > 	if( value == 0 ) {
> > 		timer->config &= ~0x800;  // disable pwm
> > 		return;
> > 	}
> > 	u32 period = -timer->reload;
> > 	if( value >= period )
> > 		timer->match = 0;
> > 	else
> > 		timer->match = value - period - 1;
> > 	timer->config |= 0x800;  // enable pwm
> > }
> > 
> > At the time I used a scope to check the exact behaviour of dmtimer pwm
> > on a dm814x.  My notes mention (when pwm enabled):
> > 	match < reload	output on continuous
> > 	match == reload	output on 1 cycle, off period-1 cycles
> > 	match == -2	output on period-1 cycles, off 1 cycle
> > 	match == -1	output freezes
> > 
> > Hope this helps  
> 
> Hi,
> 
> I think these corner cases should definitely be handled in the dmtimer driver.

Do you mean to modify the dmtimer driver itself, or the pwm-omap-dmtimer 
driver?

IIRC from the last time I was in the pwm-omap-dmtimer driver, it seemed to 
me that the 0% and 100% cases could/should be handled as simple special 
cases there. I think the dmtimer driver itself has the necessary API to the 
HW, but I'd need to re-familiarize myself with it to remember the details 
of what I was thinking. 

Actually, I did mention some thoughts on this a previous thread where
Adam Ford was using pwm-omap-dmtimer for a backlight:
 http://www.spinics.net/lists/linux-omap/msg126006.html
So it may be as simple as using PWM_OMAP_DMTIMER_TRIGGER_NONE and passing 
def_on according to whether 0 or 100% duty were requested (and polarity).


> 
> I'll try to post a fix to handle these, thanks for the original code dump.
> 
> > 
> > Matthijs
> >   
> 
> Neil
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Neil Armstrong]

On 09/12/2016 04:41 PM, David Rivshin wrote:
> On Sat, 10 Sep 2016 15:48:28 +0200
> Neil Armstrong <narmstrong@baylibre.com> wrote:
> 
>> Le 10/09/2016 05:17, Matthijs van Duin a écrit :
>>> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:  
>>>> This helps to get 100% intensity closer to "always on".
[...]
>>> }
>>>
>>> At the time I used a scope to check the exact behaviour of dmtimer pwm
>>> on a dm814x.  My notes mention (when pwm enabled):
>>> 	match < reload	output on continuous
>>> 	match == reload	output on 1 cycle, off period-1 cycles
>>> 	match == -2	output on period-1 cycles, off 1 cycle
>>> 	match == -1	output freezes
>>>
>>> Hope this helps  
>>
>> Hi,
>>
>> I think these corner cases should definitely be handled in the dmtimer driver.
> 
> Do you mean to modify the dmtimer driver itself, or the pwm-omap-dmtimer 
> driver?
> 
> IIRC from the last time I was in the pwm-omap-dmtimer driver, it seemed to 
> me that the 0% and 100% cases could/should be handled as simple special 
> cases there. I think the dmtimer driver itself has the necessary API to the 
> HW, but I'd need to re-familiarize myself with it to remember the details 
> of what I was thinking. 
> 
> Actually, I did mention some thoughts on this a previous thread where
> Adam Ford was using pwm-omap-dmtimer for a backlight:
>  http://www.spinics.net/lists/linux-omap/msg126006.html
> So it may be as simple as using PWM_OMAP_DMTIMER_TRIGGER_NONE and passing 
> def_on according to whether 0 or 100% duty were requested (and polarity).

Yes it's exactly what I was talking about.

> 
> 
>>
>> I'll try to post a fix to handle these, thanks for the original code dump.
>>
>>>
>>> Matthijs
>>>   
>>
>> Neil

Thanks,
Neil

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Neil Armstrong]

On 09/12/2016 04:41 PM, David Rivshin wrote:
> On Sat, 10 Sep 2016 15:48:28 +0200
> Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> 
>> Le 10/09/2016 05:17, Matthijs van Duin a écrit :
>>> On Mon, Sep 05, 2016 at 11:16:38AM +0200, H. Nikolaus Schaller wrote:  
>>>> This helps to get 100% intensity closer to "always on".
[...]
>>> }
>>>
>>> At the time I used a scope to check the exact behaviour of dmtimer pwm
>>> on a dm814x.  My notes mention (when pwm enabled):
>>> 	match < reload	output on continuous
>>> 	match == reload	output on 1 cycle, off period-1 cycles
>>> 	match == -2	output on period-1 cycles, off 1 cycle
>>> 	match == -1	output freezes
>>>
>>> Hope this helps  
>>
>> Hi,
>>
>> I think these corner cases should definitely be handled in the dmtimer driver.
> 
> Do you mean to modify the dmtimer driver itself, or the pwm-omap-dmtimer 
> driver?
> 
> IIRC from the last time I was in the pwm-omap-dmtimer driver, it seemed to 
> me that the 0% and 100% cases could/should be handled as simple special 
> cases there. I think the dmtimer driver itself has the necessary API to the 
> HW, but I'd need to re-familiarize myself with it to remember the details 
> of what I was thinking. 
> 
> Actually, I did mention some thoughts on this a previous thread where
> Adam Ford was using pwm-omap-dmtimer for a backlight:
>  http://www.spinics.net/lists/linux-omap/msg126006.html
> So it may be as simple as using PWM_OMAP_DMTIMER_TRIGGER_NONE and passing 
> def_on according to whether 0 or 100% duty were requested (and polarity).

Yes it's exactly what I was talking about.

> 
> 
>>
>> I'll try to post a fix to handle these, thanks for the original code dump.
>>
>>>
>>> Matthijs
>>>   
>>
>> Neil

Thanks,
Neil

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Tony Lindgren]

* H. Nikolaus Schaller <hns@goldelico.com> [160910 02:10]:
> > Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
> 
> But with the patch submitted, I just want to give the dts of a single
> device I have even designed a more reasonable value than in current
> linux/master and don't really want to make it a fundamental discussion...

So what's the verdict here on this patch? Should we wait for the driver
to get fixed?

Regards,

Tony

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [160910 02:10]:
> > Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> 
> But with the patch submitted, I just want to give the dts of a single
> device I have even designed a more reasonable value than in current
> linux/master and don't really want to make it a fundamental discussion...

So what's the verdict here on this patch? Should we wait for the driver
to get fixed?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

Hi,

> Am 14.09.2016 um 00:07 schrieb Tony Lindgren <tony@atomide.com>:
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [160910 02:10]:
>>> Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
>> 
>> But with the patch submitted, I just want to give the dts of a single
>> device I have even designed a more reasonable value than in current
>> linux/master and don't really want to make it a fundamental discussion...
> 
> So what's the verdict here on this patch? Should we wait for the driver
> to get fixed?

No, because it are indeed two issues which can (and should) be fixed independently.
This one just makes the urgency of the other bug smaller.

BR and thanks,
Nikolaus

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[H. Nikolaus Schaller]

Hi,

> Am 14.09.2016 um 00:07 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> 
> * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [160910 02:10]:
>>> Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> 
>> But with the patch submitted, I just want to give the dts of a single
>> device I have even designed a more reasonable value than in current
>> linux/master and don't really want to make it a fundamental discussion...
> 
> So what's the verdict here on this patch? Should we wait for the driver
> to get fixed?

No, because it are indeed two issues which can (and should) be fixed independently.
This one just makes the urgency of the other bug smaller.

BR and thanks,
Nikolaus

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Tony Lindgren]

* H. Nikolaus Schaller <hns@goldelico.com> [160913 21:29]:
> Hi,
> 
> > Am 14.09.2016 um 00:07 schrieb Tony Lindgren <tony@atomide.com>:
> > 
> > * H. Nikolaus Schaller <hns@goldelico.com> [160910 02:10]:
> >>> Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin@gmail.com>:
> >> 
> >> But with the patch submitted, I just want to give the dts of a single
> >> device I have even designed a more reasonable value than in current
> >> linux/master and don't really want to make it a fundamental discussion...
> > 
> > So what's the verdict here on this patch? Should we wait for the driver
> > to get fixed?
> 
> No, because it are indeed two issues which can (and should) be fixed independently.
> This one just makes the urgency of the other bug smaller.

OK applying into omap-for-v4.9/dt thanks.

Tony

Re: [PATCH] ARM: dts: omap3-gta04: reduce panel backlight PWM frequency to 83Hz

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [160913 21:29]:
> Hi,
> 
> > Am 14.09.2016 um 00:07 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> > 
> > * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [160910 02:10]:
> >>> Am 10.09.2016 um 10:20 schrieb Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> >> 
> >> But with the patch submitted, I just want to give the dts of a single
> >> device I have even designed a more reasonable value than in current
> >> linux/master and don't really want to make it a fundamental discussion...
> > 
> > So what's the verdict here on this patch? Should we wait for the driver
> > to get fixed?
> 
> No, because it are indeed two issues which can (and should) be fixed independently.
> This one just makes the urgency of the other bug smaller.

OK applying into omap-for-v4.9/dt thanks.

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html