diff mbox series

[v2,2/3] ARM: dts: rockchip: veyron: Remove 0 point from brightness-levels

Message ID 20201013010056.v2.2.I96b8d872ec51171f19274e43e96cadc092881271@changeid (mailing list archive)
State New
Headers show
Series PWM backlight interpolation adjustments | expand

Commit Message

Alexandru M Stan Oct. 13, 2020, 8:01 a.m. UTC
After the "PWM backlight interpolation adjustments" patches, the
backlight interpolation works a little differently. The way these
dts files were working before was relying on a bug (IMHO).

Remove the 0-3 range since otherwise we would have a 252 long
interpolation that would slowly go between 0 and 3, looking really bad
in userspace.

We don't need the 0% point, userspace seems to handle this just fine
because it uses the bl_power property to turn off the display.

Signed-off-by: Alexandru Stan <amstan@chromium.org>
---

 arch/arm/boot/dts/rk3288-veyron-jaq.dts    | 2 +-
 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 +-
 arch/arm/boot/dts/rk3288-veyron-tiger.dts  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Daniel Thompson Oct. 14, 2020, 2:19 p.m. UTC | #1
On Tue, Oct 13, 2020 at 01:01:02AM -0700, Alexandru Stan wrote:
> After the "PWM backlight interpolation adjustments" patches, the
> backlight interpolation works a little differently. The way these
> dts files were working before was relying on a bug (IMHO).
> 
> Remove the 0-3 range since otherwise we would have a 252 long
> interpolation that would slowly go between 0 and 3, looking really bad
> in userspace.
> 
> We don't need the 0% point, userspace seems to handle this just fine
> because it uses the bl_power property to turn off the display.
> 
> Signed-off-by: Alexandru Stan <amstan@chromium.org>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

Note also shouldn't this be patch 1 of the set. AFAICT it makes sense
whether or not the interpolation algorithm is changed.


Daniel.

> ---
> 
>  arch/arm/boot/dts/rk3288-veyron-jaq.dts    | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-tiger.dts  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> index af77ab20586d..4a148cf1defc 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> @@ -20,7 +20,7 @@ / {
>  
>  &backlight {
>  	/* Jaq panel PWM must be >= 3%, so start non-zero brightness at 8 */
> -	brightness-levels = <0 8 255>;
> +	brightness-levels = <8 255>;
>  	num-interpolated-steps = <247>;
>  };
>  
> diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
> index f8b69e0a16a0..82fc6fba9999 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
> @@ -39,7 +39,7 @@ volum_up {
>  
>  &backlight {
>  	/* Minnie panel PWM must be >= 1%, so start non-zero brightness at 3 */
> -	brightness-levels = <0 3 255>;
> +	brightness-levels = <3 255>;
>  	num-interpolated-steps = <252>;
>  };
>  
> diff --git a/arch/arm/boot/dts/rk3288-veyron-tiger.dts b/arch/arm/boot/dts/rk3288-veyron-tiger.dts
> index 069f0c2c1fdf..52a84cbe7a90 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-tiger.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-tiger.dts
> @@ -23,7 +23,7 @@ / {
>  
>  &backlight {
>  	/* Tiger panel PWM must be >= 1%, so start non-zero brightness at 3 */
> -	brightness-levels = <0 3 255>;
> +	brightness-levels = <3 255>;
>  	num-interpolated-steps = <252>;
>  };
>  
> -- 
> 2.28.0
>
Alexandru M Stan Oct. 15, 2020, 9:29 p.m. UTC | #2
On Wed, Oct 14, 2020 at 7:19 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Oct 13, 2020 at 01:01:02AM -0700, Alexandru Stan wrote:
> > After the "PWM backlight interpolation adjustments" patches, the
> > backlight interpolation works a little differently. The way these
> > dts files were working before was relying on a bug (IMHO).
> >
> > Remove the 0-3 range since otherwise we would have a 252 long
> > interpolation that would slowly go between 0 and 3, looking really bad
> > in userspace.
> >
> > We don't need the 0% point, userspace seems to handle this just fine
> > because it uses the bl_power property to turn off the display.
> >
> > Signed-off-by: Alexandru Stan <amstan@chromium.org>
>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

Thank you!

>
> Note also shouldn't this be patch 1 of the set. AFAICT it makes sense
> whether or not the interpolation algorithm is changed.

Yeah, I guess it could be. Sorry I didn't think of it that way before,
I'm used to landing things in a group.

In particular on veyron I assume it will almost be a noop without
having my driver patch (especially with the findings of 0% not being
that important).

Feel free to land this independently.

>
>
> Daniel.

Alexandru Stan (amstan)
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
index af77ab20586d..4a148cf1defc 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
@@ -20,7 +20,7 @@  / {
 
 &backlight {
 	/* Jaq panel PWM must be >= 3%, so start non-zero brightness at 8 */
-	brightness-levels = <0 8 255>;
+	brightness-levels = <8 255>;
 	num-interpolated-steps = <247>;
 };
 
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index f8b69e0a16a0..82fc6fba9999 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -39,7 +39,7 @@  volum_up {
 
 &backlight {
 	/* Minnie panel PWM must be >= 1%, so start non-zero brightness at 3 */
-	brightness-levels = <0 3 255>;
+	brightness-levels = <3 255>;
 	num-interpolated-steps = <252>;
 };
 
diff --git a/arch/arm/boot/dts/rk3288-veyron-tiger.dts b/arch/arm/boot/dts/rk3288-veyron-tiger.dts
index 069f0c2c1fdf..52a84cbe7a90 100644
--- a/arch/arm/boot/dts/rk3288-veyron-tiger.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-tiger.dts
@@ -23,7 +23,7 @@  / {
 
 &backlight {
 	/* Tiger panel PWM must be >= 1%, so start non-zero brightness at 3 */
-	brightness-levels = <0 3 255>;
+	brightness-levels = <3 255>;
 	num-interpolated-steps = <252>;
 };