diff mbox series

[v2,3/4] ARM: dts: imx6qdl-colibri: backlight pwm: Simplify inverted backlight

Message ID 20220511115911.54960-4-max.oss.09@gmail.com (mailing list archive)
State New, archived
Headers show
Series ARM: dts: imx6dl-colibri: Unify with changes to Apalis iMX6 device trees. | expand

Commit Message

Max Krummenacher May 11, 2022, 11:59 a.m. UTC 
From: Max Krummenacher <max.krummenacher@toradex.com>

Set #pwm-cells to the default 3 to gain access to the parameter
which allows inverting the PWM signal. This is useful to specify
a backlight which has its highest brightness at 0.

Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>

---

Changes in v2:
- Split to two patches as proposed by Fabio Estevam

 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Fabio Estevam May 11, 2022, 12:06 p.m. UTC | #1 
On Wed, May 11, 2022 at 8:59 AM Max Krummenacher <max.oss.09@gmail.com> wrote:

>         backlight: backlight {
>                 compatible = "pwm-backlight";
> -               brightness-levels = <0 127 191 223 239 247 251 255>;
> -               default-brightness-level = <1>;
> +               brightness-levels = <0 4 8 16 32 64 128 255>;
> +               default-brightness-level = <6>;

In this patch, you are still changing the brightness levels + passing
the polarity.

I would suggest that this patch only touches the PWM polarity.

The next patch could fix the brightness levels.
Max Krummenacher May 11, 2022, 1:32 p.m. UTC | #2 
Hi Fabio

On Wed, May 11, 2022 at 2:07 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Wed, May 11, 2022 at 8:59 AM Max Krummenacher <max.oss.09@gmail.com> wrote:
>
> >         backlight: backlight {
> >                 compatible = "pwm-backlight";
> > -               brightness-levels = <0 127 191 223 239 247 251 255>;
> > -               default-brightness-level = <1>;
> > +               brightness-levels = <0 4 8 16 32 64 128 255>;
> > +               default-brightness-level = <6>;
>
> In this patch, you are still changing the brightness levels + passing
> the polarity.
>
> I would suggest that this patch only touches the PWM polarity.

I disagree. Just setting the invert without at the same changing the
brightness-levels does
change the user experience way more than when one adapts the available
duty cycles
at the same time.

With the change to use the PWM with inverted polarity the PWM signals
is inverted to
how it was before this patch. Keeping the brightness-levels will then
have a big brightness
jump from 0 to 127 duty cycle, the other 6 steps will then be barely noticable.

I.e. before the change the brightness for level [0..7] was
['off', 128/255, 64/255, 32/255, 16/255, 8/255, 4/255, 'off'],
if one only inverts the polarity it will be
['off', 128/255, 191/255, 223/255, 239/255, 247/255, 255/255].
With the proposed patch it will be
['off', 4/255, 8/255, 16/255, 32/255, 64/255, 128/255, 255/255].

Max

> The next patch could fix the brightness levels.
Fabio Estevam May 11, 2022, 1:35 p.m. UTC | #3 
On Wed, May 11, 2022 at 10:32 AM Max Krummenacher <max.oss.09@gmail.com> wrote:

> I disagree. Just setting the invert without at the same changing the
> brightness-levels does
> change the user experience way more than when one adapts the available
> duty cycles
> at the same time.
>
> With the change to use the PWM with inverted polarity the PWM signals
> is inverted to
> how it was before this patch. Keeping the brightness-levels will then
> have a big brightness
> jump from 0 to 127 duty cycle, the other 6 steps will then be barely noticable.
>
> I.e. before the change the brightness for level [0..7] was
> ['off', 128/255, 64/255, 32/255, 16/255, 8/255, 4/255, 'off'],
> if one only inverts the polarity it will be
> ['off', 128/255, 191/255, 223/255, 239/255, 247/255, 255/255].
> With the proposed patch it will be
> ['off', 4/255, 8/255, 16/255, 32/255, 64/255, 128/255, 255/255].

Ok, please add an explanation to the commit log as to why you are
changing the brightness levels
like you did here. Thanks
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index f0908b530f86..d91fae92c90a 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -6,6 +6,7 @@ 
  */
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "Toradex Colibri iMX6DL/S Module";
@@ -13,13 +14,13 @@ 
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		brightness-levels = <0 127 191 223 239 247 251 255>;
-		default-brightness-level = <1>;
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
 		enable-gpios = <&gpio3 26 GPIO_ACTIVE_HIGH>; /* Colibri BL_ON */
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_gpio_bl_on>;
 		power-supply = <&reg_module_3v3>;
-		pwms = <&pwm3 0 5000000>;
+		pwms = <&pwm3 0 5000000 PWM_POLARITY_INVERTED>;
 		status = "disabled";
 	};
 
@@ -620,7 +621,6 @@ 
 
 /* Colibri PWM<A> */
 &pwm3 {
-	#pwm-cells = <2>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm3>;
 	status = "disabled";