diff mbox

OMAP3LOGIC: Add PWM-Backlight

Message ID 1455755412-10495-1-git-send-email-aford173@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Adam Ford Feb. 18, 2016, 12:30 a.m. UTC
The backlight pin is shared with Timer 10 PWM.  This patch allows the
pwm_bl driver to enable the pwm run by this timer to dim the backlight.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 26 +++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

Comments

Tony Lindgren Feb. 19, 2016, 6:43 p.m. UTC | #1
* Adam Ford <aford173@gmail.com> [160217 16:30]:
> The backlight pin is shared with Timer 10 PWM.  This patch allows the
> pwm_bl driver to enable the pwm run by this timer to dim the backlight.

Nice to hear the PWM is now working! Are some of the pending
fixes are still needed or is that all in Linux next now?

Also one question below..

> @@ -194,6 +203,12 @@
>  		>;
>  	};
>  
> +	pwm_pins: pinmux_pwm_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE3)       /* gpmc_ncs5.gpt_10_pwm_evt */
> +		>;
> +	};
> +
>  	led_pins: pinmux_led_pins {
>  		pinctrl-single,pins = <
>  			OMAP3_CORE1_IOPAD(0x21d8, PIN_OUTPUT | MUX_MODE4)	/* gpio_179 */
> @@ -220,7 +235,6 @@
>  
>  	backlight_pins: pinmux_backlight_pins {
>  		pinctrl-single,pins = <
> -			OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* gpmc_ncs5.gpio_56 */
>  			OMAP3_CORE1_IOPAD(0x2188, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* mcbsp4_dx.gpio_154 */
>  		>;
>  	};

What happens here if you don't set PIN_OFF_OUTPUT_LOW? Flickering
on LCD or something?

Regards,

Tony
--
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
Adam Ford Feb. 19, 2016, 7:40 p.m. UTC | #2
The PWM stuff worked with what was in linux-next.  I didn't have to do
any extra patches or anything.  (Thanks to David Rivshin for helping
me figure that out)

PIN_OFF_OUTPUT_LOW was set to minimize power consumption during sleep.
Looking at Logic PD's stock BSP, they have those pins enabled, and it
seemed to help me cut power with "echo mem > /sys/power/state"

If you would prefer that I not use that, I can certainly look at some
other options.  Just let me know, and I can adjust the patch
accordingly.

adam

On Fri, Feb 19, 2016 at 12:43 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Adam Ford <aford173@gmail.com> [160217 16:30]:
>> The backlight pin is shared with Timer 10 PWM.  This patch allows the
>> pwm_bl driver to enable the pwm run by this timer to dim the backlight.
>
> Nice to hear the PWM is now working! Are some of the pending
> fixes are still needed or is that all in Linux next now?
>
> Also one question below..
>
>> @@ -194,6 +203,12 @@
>>               >;
>>       };
>>
>> +     pwm_pins: pinmux_pwm_pins {
>> +             pinctrl-single,pins = <
>> +                     OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE3)       /* gpmc_ncs5.gpt_10_pwm_evt */
>> +             >;
>> +     };
>> +
>>       led_pins: pinmux_led_pins {
>>               pinctrl-single,pins = <
>>                       OMAP3_CORE1_IOPAD(0x21d8, PIN_OUTPUT | MUX_MODE4)       /* gpio_179 */
>> @@ -220,7 +235,6 @@
>>
>>       backlight_pins: pinmux_backlight_pins {
>>               pinctrl-single,pins = <
>> -                     OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* gpmc_ncs5.gpio_56 */
>>                       OMAP3_CORE1_IOPAD(0x2188, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* mcbsp4_dx.gpio_154 */
>>               >;
>>       };
>
> What happens here if you don't set PIN_OFF_OUTPUT_LOW? Flickering
> on LCD or something?
>
> Regards,
>
> Tony
--
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
Tony Lindgren Feb. 19, 2016, 7:50 p.m. UTC | #3
* Adam Ford <aford173@gmail.com> [160219 11:40]:
> The PWM stuff worked with what was in linux-next.  I didn't have to do
> any extra patches or anything.  (Thanks to David Rivshin for helping
> me figure that out)

OK nice.

> PIN_OFF_OUTPUT_LOW was set to minimize power consumption during sleep.
> Looking at Logic PD's stock BSP, they have those pins enabled, and it
> seemed to help me cut power with "echo mem > /sys/power/state"

OK makes sense to me thanks.

> If you would prefer that I not use that, I can certainly look at some
> other options.  Just let me know, and I can adjust the patch
> accordingly.

No need to, there's other way to do it AFAIK :)

Regards,

Tony
--
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
David Rivshin (Allworx) Feb. 20, 2016, 12:11 a.m. UTC | #4
On Wed, 17 Feb 2016 18:30:12 -0600
Adam Ford <aford173@gmail.com> wrote:

> The backlight pin is shared with Timer 10 PWM.  This patch allows the
> pwm_bl driver to enable the pwm run by this timer to dim the backlight.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
[...]
>  	bl: backlight {
> -		compatible = "gpio-backlight";
> +		compatible = "pwm-backlight";
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&backlight_pins>;
> -
> -		gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, /* gpio_154 */
> -			<&gpio2 24 GPIO_ACTIVE_HIGH>; /* gpio_56 */
> -		default-on;
> +		pwms = <&pwm10 0 5000000 0>;
> +		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
> +		default-brightness-level = <7>;
> +		enable-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; /* gpio_154 */
>  	};
[...]

One thing to note: omap-dmtimer-pwm can't do a perfect 0 or 100% duty cycle. 
I think with the code in linux-next right now it will just configure the 
DMTIMER in a way that the TRM says is illegal, and with my [PATCH 2/4] it 
will set the lowest/highest possible duty-cycle (1-fclk cycle per period).
Assuming you're using the 32768Hz fclk for timer10, that would mean the 
lowest effective duty cycle would be about 0.6%, and the highest 99.4%. If 
you're using a faster fclk, then the error would be proportionally less. 
Just wanted to mention that in case it's an issue for your backlight.

FYI, I've seen other PWM drivers use a trick for the 0 and 100% cases where 
they actually force the largest possible period. This then dilutes the 
unwanted 1-cycle pulse as much as possible. I see no reason that couldn't 
be done for omap-dmtimer-pwm as well. 

Looking at the TRM, it may also be possible to turn off the trigger, and 
use the TCLR.SCPWM bit to force a perfect 0 or 100% duty cycle. 
--
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
Adam Ford Feb. 20, 2016, 12:15 a.m. UTC | #5
I did notice that 0 and 100 were not perfect, but it doesn't bother me
because the visual effect was more important to me than precision.  If
you think it would be better, I could make the limits 1 and 99 for the
extreme limits.

adam

On Fri, Feb 19, 2016 at 6:11 PM, David Rivshin (Allworx)
<drivshin.allworx@gmail.com> wrote:
> On Wed, 17 Feb 2016 18:30:12 -0600
> Adam Ford <aford173@gmail.com> wrote:
>
>> The backlight pin is shared with Timer 10 PWM.  This patch allows the
>> pwm_bl driver to enable the pwm run by this timer to dim the backlight.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>> ---
> [...]
>>       bl: backlight {
>> -             compatible = "gpio-backlight";
>> +             compatible = "pwm-backlight";
>>               pinctrl-names = "default";
>>               pinctrl-0 = <&backlight_pins>;
>> -
>> -             gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, /* gpio_154 */
>> -                     <&gpio2 24 GPIO_ACTIVE_HIGH>; /* gpio_56 */
>> -             default-on;
>> +             pwms = <&pwm10 0 5000000 0>;
>> +             brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
>> +             default-brightness-level = <7>;
>> +             enable-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; /* gpio_154 */
>>       };
> [...]
>
> One thing to note: omap-dmtimer-pwm can't do a perfect 0 or 100% duty cycle.
> I think with the code in linux-next right now it will just configure the
> DMTIMER in a way that the TRM says is illegal, and with my [PATCH 2/4] it
> will set the lowest/highest possible duty-cycle (1-fclk cycle per period).
> Assuming you're using the 32768Hz fclk for timer10, that would mean the
> lowest effective duty cycle would be about 0.6%, and the highest 99.4%. If
> you're using a faster fclk, then the error would be proportionally less.
> Just wanted to mention that in case it's an issue for your backlight.
>
> FYI, I've seen other PWM drivers use a trick for the 0 and 100% cases where
> they actually force the largest possible period. This then dilutes the
> unwanted 1-cycle pulse as much as possible. I see no reason that couldn't
> be done for omap-dmtimer-pwm as well.
>
> Looking at the TRM, it may also be possible to turn off the trigger, and
> use the TCLR.SCPWM bit to force a perfect 0 or 100% duty cycle.
--
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
David Rivshin (Allworx) Feb. 20, 2016, 2:15 a.m. UTC | #6
On Fri, 19 Feb 2016 18:15:58 -0600
Adam Ford <aford173@gmail.com> wrote:

> I did notice that 0 and 100 were not perfect, but it doesn't bother me
> because the visual effect was more important to me than precision.  If
> you think it would be better, I could make the limits 1 and 99 for the
> extreme limits.

I think your devicetree itself is fine. If better behavior is required
improving omap-dmtimer-pwm would probably be the right place to fix it. 
I think the hardware is capable if programmed in a special way, so we'd 
just need to check for those special-case values to handle differently.

Also, If the pwm-backlight binding documentation is correct, the highest 
value will always ask for 100% duty cycle, so changing that to 99 wouldn't 
have an effect (and I doubt that anyone could discern 99.4% vs 100% anyways).
I was more concerned with the low end, as I would be surprised if 0.6% 
brightness is noticeable. At least some LEDs are bright enough to be easily 
visible even at such low PWM. But if it's OK in your case, I wouldn't
worry about it.
 
> On Fri, Feb 19, 2016 at 6:11 PM, David Rivshin (Allworx)
> <drivshin.allworx@gmail.com> wrote:
> > On Wed, 17 Feb 2016 18:30:12 -0600
> > Adam Ford <aford173@gmail.com> wrote:
> >  
> >> The backlight pin is shared with Timer 10 PWM.  This patch allows the
> >> pwm_bl driver to enable the pwm run by this timer to dim the backlight.
> >>
> >> Signed-off-by: Adam Ford <aford173@gmail.com>
> >> ---  
> > [...]  
> >>       bl: backlight {
> >> -             compatible = "gpio-backlight";
> >> +             compatible = "pwm-backlight";
> >>               pinctrl-names = "default";
> >>               pinctrl-0 = <&backlight_pins>;
> >> -
> >> -             gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, /* gpio_154 */
> >> -                     <&gpio2 24 GPIO_ACTIVE_HIGH>; /* gpio_56 */
> >> -             default-on;
> >> +             pwms = <&pwm10 0 5000000 0>;
> >> +             brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
> >> +             default-brightness-level = <7>;
> >> +             enable-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; /* gpio_154 */
> >>       };  
> > [...]
> >
> > One thing to note: omap-dmtimer-pwm can't do a perfect 0 or 100% duty cycle.
> > I think with the code in linux-next right now it will just configure the
> > DMTIMER in a way that the TRM says is illegal, and with my [PATCH 2/4] it
> > will set the lowest/highest possible duty-cycle (1-fclk cycle per period).
> > Assuming you're using the 32768Hz fclk for timer10, that would mean the
> > lowest effective duty cycle would be about 0.6%, and the highest 99.4%. If
> > you're using a faster fclk, then the error would be proportionally less.
> > Just wanted to mention that in case it's an issue for your backlight.
> >
> > FYI, I've seen other PWM drivers use a trick for the 0 and 100% cases where
> > they actually force the largest possible period. This then dilutes the
> > unwanted 1-cycle pulse as much as possible. I see no reason that couldn't
> > be done for omap-dmtimer-pwm as well.
> >
> > Looking at the TRM, it may also be possible to turn off the trigger, and
> > use the TCLR.SCPWM bit to force a perfect 0 or 100% duty cycle.  

--
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
Tony Lindgren Feb. 22, 2016, 6:18 p.m. UTC | #7
* Tony Lindgren <tony@atomide.com> [160219 11:50]:
> * Adam Ford <aford173@gmail.com> [160219 11:40]:
> > The PWM stuff worked with what was in linux-next.  I didn't have to do
> > any extra patches or anything.  (Thanks to David Rivshin for helping
> > me figure that out)
> 
> OK nice.
> 
> > PIN_OFF_OUTPUT_LOW was set to minimize power consumption during sleep.
> > Looking at Logic PD's stock BSP, they have those pins enabled, and it
> > seemed to help me cut power with "echo mem > /sys/power/state"
> 
> OK makes sense to me thanks.
> 
> > If you would prefer that I not use that, I can certainly look at some
> > other options.  Just let me know, and I can adjust the patch
> > accordingly.
> 
> No need to, there's other way to do it AFAIK :)

Applying this into omap-for-v4.6/dts thanks.

Tony
--
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
diff mbox

Patch

diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
index 4b00b58..1a97dda 100644
--- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
+++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
@@ -71,6 +71,15 @@ 
 			linux,default-trigger = "none";
 		};
 	};
+
+	pwm10: dmtimer-pwm@10 {
+		compatible = "ti,omap-dmtimer-pwm";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pwm_pins>;
+		ti,timers = <&timer10>;
+		#pwm-cells = <3>;
+	};
+
 };
 
 &vaux1 {
@@ -166,13 +175,13 @@ 
 	};
 
 	bl: backlight {
-		compatible = "gpio-backlight";
+		compatible = "pwm-backlight";
 		pinctrl-names = "default";
 		pinctrl-0 = <&backlight_pins>;
-
-		gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, /* gpio_154 */
-			<&gpio2 24 GPIO_ACTIVE_HIGH>; /* gpio_56 */
-		default-on;
+		pwms = <&pwm10 0 5000000 0>;
+		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
+		default-brightness-level = <7>;
+		enable-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; /* gpio_154 */
 	};
 };
 
@@ -194,6 +203,12 @@ 
 		>;
 	};
 
+	pwm_pins: pinmux_pwm_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE3)       /* gpmc_ncs5.gpt_10_pwm_evt */
+		>;
+	};
+
 	led_pins: pinmux_led_pins {
 		pinctrl-single,pins = <
 			OMAP3_CORE1_IOPAD(0x21d8, PIN_OUTPUT | MUX_MODE4)	/* gpio_179 */
@@ -220,7 +235,6 @@ 
 
 	backlight_pins: pinmux_backlight_pins {
 		pinctrl-single,pins = <
-			OMAP3_CORE1_IOPAD(0x20B8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* gpmc_ncs5.gpio_56 */
 			OMAP3_CORE1_IOPAD(0x2188, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE4)       /* mcbsp4_dx.gpio_154 */
 		>;
 	};