[PATCHv3] arm64: dts: allwinner: a64: teres-i: enable backlight

[PATCHv3] arm64: dts: allwinner: a64: teres-i: enable backlight

[Harald Geyer]

Enable pwm and add a pretty standard backlight node.

The regulator is always on, but we include it anyway, because it is
required by the binding document.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
The backlight node got dropped from the initial submission of the
teres-i DT, because PWM support wasn't available in time, and I
kind of forgot to resubmit once PWM was in. Sorry about that.

While testing this patch I noticed, that sometimes on boot the
brightness is set to max_brightness instead of the default specified
in DT. I couldn't reproduce it reliably, but it seems to be related
to changing the pwm period. I guess the logic trying to detect
the brightness set by the boot loader gets confused in some corner
case if the pwm periods don't match. However unbinding and rebinding
the device to the driver always made it go to the proper default
brightness, so the problem is clearly not in the DT.

If the theory above is true, then it implies that the version of
u-boot running on my laptop doesn't completely overwrite all pwm
parameters. As this might be specific to my installation, I didn't
mention the issue in the commit message.

Changes since v2:
 * Drop all the other stuff that got merged a year ago.
 * Rebased on current sunxi/for-next branch
 * Add power-supply property (just to conform to the binding)
 * Use slightly different brightness-levels

Actually I tried omitting brightness-levels completely, as it is now
optional. However automatic calculation gives unreasonable results
(depending on the exact value of pwm period). A report has been sent
to the author of the code.


 .../arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 7b7b14ba58e6..2a78932d5d3b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -21,6 +21,15 @@
 		serial0 = &uart0;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm 0 50000 0>;
+		power-supply = <&reg_dcdc1>;
+		brightness-levels = <0 5 10 15 20 30 40 55 70 85 100>;
+		default-brightness-level = <4>;
+		enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 
@@ -131,6 +140,10 @@
 	status = "okay";
 };
 
+&pwm {
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3] arm64: dts: allwinner: a64: teres-i: enable backlight

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2866 bytes --]

Hi Harald,

On Fri, Feb 08, 2019 at 02:16:29PM +0000, Harald Geyer wrote:
> Enable pwm and add a pretty standard backlight node.
> 
> The regulator is always on, but we include it anyway, because it is
> required by the binding document.
> 
> Signed-off-by: Harald Geyer <harald@ccbib.org>
> ---
> The backlight node got dropped from the initial submission of the
> teres-i DT, because PWM support wasn't available in time, and I
> kind of forgot to resubmit once PWM was in. Sorry about that.
> 
> While testing this patch I noticed, that sometimes on boot the
> brightness is set to max_brightness instead of the default specified
> in DT. I couldn't reproduce it reliably, but it seems to be related
> to changing the pwm period. I guess the logic trying to detect
> the brightness set by the boot loader gets confused in some corner
> case if the pwm periods don't match. However unbinding and rebinding
> the device to the driver always made it go to the proper default
> brightness, so the problem is clearly not in the DT.
> 
> If the theory above is true, then it implies that the version of
> u-boot running on my laptop doesn't completely overwrite all pwm
> parameters. As this might be specific to my installation, I didn't
> mention the issue in the commit message.
> 
> Changes since v2:
>  * Drop all the other stuff that got merged a year ago.
>  * Rebased on current sunxi/for-next branch
>  * Add power-supply property (just to conform to the binding)
>  * Use slightly different brightness-levels
> 
> Actually I tried omitting brightness-levels completely, as it is now
> optional. However automatic calculation gives unreasonable results
> (depending on the exact value of pwm period). A report has been sent
> to the author of the code.
> 
> 
>  .../arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> index 7b7b14ba58e6..2a78932d5d3b 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -21,6 +21,15 @@
>  		serial0 = &uart0;
>  	};
>  
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm 0 50000 0>;
> +		power-supply = <&reg_dcdc1>;
> +		brightness-levels = <0 5 10 15 20 30 40 55 70 85 100>;

The backlight perceived brightness should increase lineary with each
step, which means that if you have ten steps, each step should
increase the perceived brightness by 10%. The eye being what it is, a
exponential sequence is usually a much better approximation.

It looks good otherwise, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel