linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] PWM backlight interpolation adjustments
@ 2020-10-13  8:01 Alexandru Stan
  2020-10-13  8:01 ` [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels Alexandru Stan
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Stan @ 2020-10-13  8:01 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones,
	Daniel Thompson, Jingoo Han, Bartlomiej Zolnierkiewicz,
	Heiko Stuebner, Rob Herring, Andy Gross, Bjorn Andersson
  Cc: Douglas Anderson, Enric Balletbo i Serra, Matthias Kaehlcke,
	Alexandru Stan, devicetree, dri-devel, linux-arm-kernel,
	linux-arm-msm, linux-fbdev, linux-kernel, linux-pwm,
	linux-rockchip

I was trying to adjust the brightness-levels for the trogdor boards:
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2291209
Like on a lot of panels, trogdor's low end needs to be cropped,
and now that we have the interpolation stuff I wanted to make use of it
and bake in even the curve that's customary to have on chromebooks.

I found the current behavior of the pwm_bl driver a little unintuitive
and non-linear. See patch 1 for a suggested fix for this.

A few veyron dts files were relying on this (perhaps weird) behavior.
Those devices also want a minimum brightness like trogdor, so changed
them to use the new way.

Finally, given that trogdor's dts is part of linux-next now, add the
brightness-levels to it, since that's the original reason I was looking at
this.

Changes in v2:
- Fixed type promotion in the driver
- Removed "backlight: pwm_bl: Artificially add 0% during interpolation",
userspace works just fine without it because it already knows how to use
bl_power for turning off the display.
- Added brightness-levels to trogdor as well, now the dts is upstream.


Alexandru Stan (3):
  backlight: pwm_bl: Fix interpolation
  ARM: dts: rockchip: veyron: Remove 0 point from brightness-levels
  arm64: dts: qcom: trogdor: Add brightness-levels

 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 +-
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |  9 +++
 drivers/video/backlight/pwm_bl.c             | 70 +++++++++-----------
 5 files changed, 43 insertions(+), 42 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels
  2020-10-13  8:01 [PATCH v2 0/3] PWM backlight interpolation adjustments Alexandru Stan
@ 2020-10-13  8:01 ` Alexandru Stan
  2020-10-13 16:28   ` Doug Anderson
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Stan @ 2020-10-13  8:01 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones,
	Daniel Thompson, Jingoo Han, Bartlomiej Zolnierkiewicz,
	Heiko Stuebner, Rob Herring, Andy Gross, Bjorn Andersson
  Cc: Douglas Anderson, Enric Balletbo i Serra, Matthias Kaehlcke,
	Alexandru Stan, devicetree, linux-arm-msm, linux-kernel

Now that we have better interpolation for the backlight
("backlight: pwm_bl: Fix interpolation"), we can now add the curve to
the trogdor boards, being careful to crop the low end.

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

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index bf875589d364..ccdabc6c4994 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -179,6 +179,15 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
 	backlight: backlight {
 		compatible = "pwm-backlight";
 
+		/* The panels don't seem to like anything below ~ 5% */
+		brightness-levels = <
+			196 256 324 400 484 576 676 784 900 1024 1156 1296
+			1444 1600 1764 1936 2116 2304 2500 2704 2916 3136
+			3364 3600 3844 4096
+		>;
+		num-interpolated-steps = <64>;
+		default-brightness-level = <951>;
+
 		pwms = <&cros_ec_pwm 1>;
 		enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
 		power-supply = <&ppvar_sys>;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels
  2020-10-13  8:01 ` [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels Alexandru Stan
@ 2020-10-13 16:28   ` Doug Anderson
  2020-10-14 11:33     ` Daniel Thompson
  0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2020-10-13 16:28 UTC (permalink / raw)
  To: Alexandru Stan
  Cc: Thierry Reding, Uwe Kleine-König, Lee Jones,
	Daniel Thompson, Jingoo Han, Bartlomiej Zolnierkiewicz,
	Heiko Stuebner, Rob Herring, Andy Gross, Bjorn Andersson,
	Enric Balletbo i Serra, Matthias Kaehlcke,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Tue, Oct 13, 2020 at 1:01 AM Alexandru Stan <amstan@chromium.org> wrote:
>
> Now that we have better interpolation for the backlight
> ("backlight: pwm_bl: Fix interpolation"), we can now add the curve to
> the trogdor boards, being careful to crop the low end.

Just to make it clear, the patch this depends on hasn't landed yet.
Presumably it will land in the v5.10 timeframe?  That means that
without extra coordination this patch can target v5.11.


> Signed-off-by: Alexandru Stan <amstan@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index bf875589d364..ccdabc6c4994 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -179,6 +179,15 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
>         backlight: backlight {
>                 compatible = "pwm-backlight";
>
> +               /* The panels don't seem to like anything below ~ 5% */
> +               brightness-levels = <
> +                       196 256 324 400 484 576 676 784 900 1024 1156 1296
> +                       1444 1600 1764 1936 2116 2304 2500 2704 2916 3136
> +                       3364 3600 3844 4096
> +               >;
> +               num-interpolated-steps = <64>;
> +               default-brightness-level = <951>;

I haven't done lots of digging here, but this matches what Alexandru
and Matthias agreed upon for the downstream tree and seems sane.
Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels
  2020-10-13 16:28   ` Doug Anderson
@ 2020-10-14 11:33     ` Daniel Thompson
  2020-10-14 13:51       ` Doug Anderson
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Thompson @ 2020-10-14 11:33 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Alexandru Stan, Thierry Reding, Uwe Kleine-König, Lee Jones,
	Jingoo Han, Bartlomiej Zolnierkiewicz, Heiko Stuebner,
	Rob Herring, Andy Gross, Bjorn Andersson, Enric Balletbo i Serra,
	Matthias Kaehlcke,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

On Tue, Oct 13, 2020 at 09:28:38AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Oct 13, 2020 at 1:01 AM Alexandru Stan <amstan@chromium.org> wrote:
> >
> > Now that we have better interpolation for the backlight
> > ("backlight: pwm_bl: Fix interpolation"), we can now add the curve to
> > the trogdor boards, being careful to crop the low end.
> 
> Just to make it clear, the patch this depends on hasn't landed yet.
> Presumably it will land in the v5.10 timeframe?  That means that
> without extra coordination this patch can target v5.11.

You're talking about patch 1 from this set? Despite the title I view
the patch as changing policy (albeit one that does also fix some annoying
quantization errors at the same time) so it's not necessarily a
candidate for merging outside the merge window (I've not checked with
Lee but I think it likely the shutter is already down for features).

Moreover I'm not clear why there a dependency here that would stop the
changes landing in different trees.


Daniel.


> > Signed-off-by: Alexandru Stan <amstan@chromium.org>
> > ---
> >
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > index bf875589d364..ccdabc6c4994 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > @@ -179,6 +179,15 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> >         backlight: backlight {
> >                 compatible = "pwm-backlight";
> >
> > +               /* The panels don't seem to like anything below ~ 5% */
> > +               brightness-levels = <
> > +                       196 256 324 400 484 576 676 784 900 1024 1156 1296
> > +                       1444 1600 1764 1936 2116 2304 2500 2704 2916 3136
> > +                       3364 3600 3844 4096
> > +               >;
> > +               num-interpolated-steps = <64>;
> > +               default-brightness-level = <951>;
> 
> I haven't done lots of digging here, but this matches what Alexandru
> and Matthias agreed upon for the downstream tree and seems sane.
> Thus:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels
  2020-10-14 11:33     ` Daniel Thompson
@ 2020-10-14 13:51       ` Doug Anderson
  2020-10-15  9:15         ` Daniel Thompson
  0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2020-10-14 13:51 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Alexandru Stan, Thierry Reding, Uwe Kleine-König, Lee Jones,
	Jingoo Han, Bartlomiej Zolnierkiewicz, Heiko Stuebner,
	Rob Herring, Andy Gross, Bjorn Andersson, Enric Balletbo i Serra,
	Matthias Kaehlcke,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Wed, Oct 14, 2020 at 4:33 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Oct 13, 2020 at 09:28:38AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Oct 13, 2020 at 1:01 AM Alexandru Stan <amstan@chromium.org> wrote:
> > >
> > > Now that we have better interpolation for the backlight
> > > ("backlight: pwm_bl: Fix interpolation"), we can now add the curve to
> > > the trogdor boards, being careful to crop the low end.
> >
> > Just to make it clear, the patch this depends on hasn't landed yet.
> > Presumably it will land in the v5.10 timeframe?  That means that
> > without extra coordination this patch can target v5.11.
>
> You're talking about patch 1 from this set? Despite the title I view
> the patch as changing policy (albeit one that does also fix some annoying
> quantization errors at the same time) so it's not necessarily a
> candidate for merging outside the merge window (I've not checked with
> Lee but I think it likely the shutter is already down for features).

Ugh, I'm off by one.  :(  Right.  New features prepared for v5.10
should already have been baking in linuxnext and merge requests have
already started flowing towards Linus.  After v5.10-rc1 then it'd just
fixes and this doesn't really qualify.  So the timing dictates that
patch #1 will land in v5.11, not v5.10.


> Moreover I'm not clear why there a dependency here that would stop the
> changes landing in different trees.

I haven't tried Alexandru's device tree patch without the associated
code changes, but I guess I just assumed that it would make a really
ugly (non)ideal backlight curve and we'd be better off with what we
currently have (AKA no curve specified at all).

Hrm, thinking about it, I guess the worst case is a slightly non-ideal
backlight curve and it would be all good in the final v5.11 which
would have the device tree and code changes, so you're right that both
the code and device tree could target v5.11 without anything too
bad...


> Daniel.
>
>
> > > Signed-off-by: Alexandru Stan <amstan@chromium.org>
> > > ---
> > >
> > >  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > index bf875589d364..ccdabc6c4994 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > @@ -179,6 +179,15 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> > >         backlight: backlight {
> > >                 compatible = "pwm-backlight";
> > >
> > > +               /* The panels don't seem to like anything below ~ 5% */
> > > +               brightness-levels = <
> > > +                       196 256 324 400 484 576 676 784 900 1024 1156 1296
> > > +                       1444 1600 1764 1936 2116 2304 2500 2704 2916 3136
> > > +                       3364 3600 3844 4096
> > > +               >;
> > > +               num-interpolated-steps = <64>;
> > > +               default-brightness-level = <951>;
> >
> > I haven't done lots of digging here, but this matches what Alexandru
> > and Matthias agreed upon for the downstream tree and seems sane.
> > Thus:
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels
  2020-10-14 13:51       ` Doug Anderson
@ 2020-10-15  9:15         ` Daniel Thompson
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Thompson @ 2020-10-15  9:15 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Alexandru Stan, Thierry Reding, Uwe Kleine-König, Lee Jones,
	Jingoo Han, Bartlomiej Zolnierkiewicz, Heiko Stuebner,
	Rob Herring, Andy Gross, Bjorn Andersson, Enric Balletbo i Serra,
	Matthias Kaehlcke,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

On Wed, Oct 14, 2020 at 06:51:19AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Wed, Oct 14, 2020 at 4:33 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Tue, Oct 13, 2020 at 09:28:38AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Tue, Oct 13, 2020 at 1:01 AM Alexandru Stan <amstan@chromium.org> wrote:
> > > >
> > > > Now that we have better interpolation for the backlight
> > > > ("backlight: pwm_bl: Fix interpolation"), we can now add the curve to
> > > > the trogdor boards, being careful to crop the low end.
> > >
> > > Just to make it clear, the patch this depends on hasn't landed yet.
> > > Presumably it will land in the v5.10 timeframe?  That means that
> > > without extra coordination this patch can target v5.11.
> >
> > You're talking about patch 1 from this set? Despite the title I view
> > the patch as changing policy (albeit one that does also fix some annoying
> > quantization errors at the same time) so it's not necessarily a
> > candidate for merging outside the merge window (I've not checked with
> > Lee but I think it likely the shutter is already down for features).
> 
> Ugh, I'm off by one.  :(  Right.  New features prepared for v5.10
> should already have been baking in linuxnext and merge requests have
> already started flowing towards Linus.  After v5.10-rc1 then it'd just
> fixes and this doesn't really qualify.  So the timing dictates that
> patch #1 will land in v5.11, not v5.10.
> 
> 
> > Moreover I'm not clear why there a dependency here that would stop the
> > changes landing in different trees.
> 
> I haven't tried Alexandru's device tree patch without the associated
> code changes, but I guess I just assumed that it would make a really
> ugly (non)ideal backlight curve and we'd be better off with what we
> currently have (AKA no curve specified at all).
> 
> Hrm, thinking about it, I guess the worst case is a slightly non-ideal
> backlight curve and it would be all good in the final v5.11 which
> would have the device tree and code changes, so you're right that both
> the code and device tree could target v5.11 without anything too
> bad...

Excellent. I'll try to remember this for v3 so I can get my Acked-by
versus Reviewed-by tags correct ;-) .


Daniel.


> 
> 
> > Daniel.
> >
> >
> > > > Signed-off-by: Alexandru Stan <amstan@chromium.org>
> > > > ---
> > > >
> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > > index bf875589d364..ccdabc6c4994 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > > @@ -179,6 +179,15 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> > > >         backlight: backlight {
> > > >                 compatible = "pwm-backlight";
> > > >
> > > > +               /* The panels don't seem to like anything below ~ 5% */
> > > > +               brightness-levels = <
> > > > +                       196 256 324 400 484 576 676 784 900 1024 1156 1296
> > > > +                       1444 1600 1764 1936 2116 2304 2500 2704 2916 3136
> > > > +                       3364 3600 3844 4096
> > > > +               >;
> > > > +               num-interpolated-steps = <64>;
> > > > +               default-brightness-level = <951>;
> > >
> > > I haven't done lots of digging here, but this matches what Alexandru
> > > and Matthias agreed upon for the downstream tree and seems sane.
> > > Thus:
> > >
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-15  9:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13  8:01 [PATCH v2 0/3] PWM backlight interpolation adjustments Alexandru Stan
2020-10-13  8:01 ` [PATCH v2 3/3] arm64: dts: qcom: trogdor: Add brightness-levels Alexandru Stan
2020-10-13 16:28   ` Doug Anderson
2020-10-14 11:33     ` Daniel Thompson
2020-10-14 13:51       ` Doug Anderson
2020-10-15  9:15         ` Daniel Thompson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).