From: Matthias Kaehlcke <mka@chromium.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: Lee Jones <lee.jones@linaro.org>,
Daniel Thompson <daniel.thompson@linaro.org>,
Jingoo Han <jingoohan1@gmail.com>,
Jacek Anaszewski <jacek.anaszewski@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Thierry Reding <thierry.reding@gmail.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Enric Balletbo i Serra <enric.balletbo@collabora.com>,
dri-devel@lists.freedesktop.org, linux-leds@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pwm@vger.kernel.org, linux-fbdev@vger.kernel.org,
Douglas Anderson <dianders@chromium.org>,
Brian Norris <briannorris@chromium.org>
Subject: Re: [PATCH 2/2] backlight: pwm_bl: Get number of brightness levels for CIE 1931 from the device tree
Date: Tue, 11 Jun 2019 21:58:59 +0000 [thread overview]
Message-ID: <20190611215859.GF137143@google.com> (raw)
In-Reply-To: <20190611101843.GD7526@amd>
Hi Pavel,
On Tue, Jun 11, 2019 at 12:18:43PM +0200, Pavel Machek wrote:
> On Mon 2019-06-10 16:37:39, Matthias Kaehlcke wrote:
> > Commit 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED
> > linearly to human eye") uses pwm_period / hweight32(pwm_period) as
> > as heuristic to determine the number of brightness levels when the DT
> > doesn't provide a brightness level table. This heuristic is broken
> > and can result in excessively large brightness tables.
> >
> > Instead of using the heuristic try to retrieve the number of
> > brightness levels from the device tree (property 'max-brightness'
> > + 1). If the value is not specified use a default of 256 levels.
> >
> > Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to human eye")
>
> I don't think this one is suitable for stable. I'm pretty sure the
> heuristics works well for many boards, and you just replaced it with
> another heuristics ("256").
whether the patch is suitable for stable/upstream is certainly
debatable, in any case I'd argue the current heuristic is bogus and
works by accident or at a cost:
nlevels = period / hweight(period)
w/ period = 131071 ns (0x1FFFF)
nlevels = 131071 / 17 = 7710
w/ period = 131072 ns (0x20000)
nlevels = 131072 / 1 = 131072
and some PWMs use significantly higher periods like 1 ms or 10 ms.
WARNING: multiple messages have this Message-ID (diff)
From: Matthias Kaehlcke <mka@chromium.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: Lee Jones <lee.jones@linaro.org>,
Daniel Thompson <daniel.thompson@linaro.org>,
Jingoo Han <jingoohan1@gmail.com>,
Jacek Anaszewski <jacek.anaszewski@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Thierry Reding <thierry.reding@gmail.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Enric Balletbo i Serra <enric.balletbo@collabora.com>,
dri-devel@lists.freedesktop.org, linux-leds@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pwm@vger.kernel.org, linux-fbdev@vger.kernel.org,
Douglas Anderson <dianders@chromium.org>,
Brian Norris <briannorris@chromium.org>
Subject: Re: [PATCH 2/2] backlight: pwm_bl: Get number of brightness levels for CIE 1931 from the device tree
Date: Tue, 11 Jun 2019 14:58:59 -0700 [thread overview]
Message-ID: <20190611215859.GF137143@google.com> (raw)
In-Reply-To: <20190611101843.GD7526@amd>
Hi Pavel,
On Tue, Jun 11, 2019 at 12:18:43PM +0200, Pavel Machek wrote:
> On Mon 2019-06-10 16:37:39, Matthias Kaehlcke wrote:
> > Commit 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED
> > linearly to human eye") uses pwm_period / hweight32(pwm_period) as
> > as heuristic to determine the number of brightness levels when the DT
> > doesn't provide a brightness level table. This heuristic is broken
> > and can result in excessively large brightness tables.
> >
> > Instead of using the heuristic try to retrieve the number of
> > brightness levels from the device tree (property 'max-brightness'
> > + 1). If the value is not specified use a default of 256 levels.
> >
> > Fixes: 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED linearly to human eye")
>
> I don't think this one is suitable for stable. I'm pretty sure the
> heuristics works well for many boards, and you just replaced it with
> another heuristics ("256").
whether the patch is suitable for stable/upstream is certainly
debatable, in any case I'd argue the current heuristic is bogus and
works by accident or at a cost:
nlevels = period / hweight(period)
w/ period = 131071 ns (0x1FFFF)
nlevels = 131071 / 17 = 7710
w/ period = 131072 ns (0x20000)
nlevels = 131072 / 1 = 131072
and some PWMs use significantly higher periods like 1 ms or 10 ms.
next prev parent reply other threads:[~2019-06-11 21:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-28 11:16 [PATCH 1/2] dt-bindings: pwm-backlight: Add pwm-delay-us property Enric Balletbo i Serra
2017-06-28 11:16 ` Enric Balletbo i Serra
2017-06-28 11:16 ` [PATCH 2/2] pwm-backlight: Add support for " Enric Balletbo i Serra
2017-06-28 11:16 ` Enric Balletbo i Serra
2017-06-28 13:16 ` [PATCH 1/2] dt-bindings: pwm-backlight: Add " Daniel Thompson
2017-06-28 13:16 ` Daniel Thompson
2017-06-28 13:30 ` Pavel Machek
2017-06-28 13:30 ` Pavel Machek
2017-06-28 14:15 ` Enric Balletbo Serra
2017-06-28 14:15 ` Enric Balletbo Serra
2017-06-29 13:07 ` Daniel Thompson
2017-06-29 13:07 ` Daniel Thompson
2019-06-10 23:37 ` [PATCH 1/2] dt-bindings: pwm-backlight: Add 'max-brightness' property Matthias Kaehlcke
2019-06-10 23:37 ` Matthias Kaehlcke
2019-06-10 23:37 ` [PATCH 2/2] backlight: pwm_bl: Get number of brightness levels for CIE 1931 from the device tree Matthias Kaehlcke
2019-06-10 23:37 ` Matthias Kaehlcke
2019-06-11 10:18 ` Pavel Machek
2019-06-11 10:18 ` Pavel Machek
2019-06-11 10:18 ` Pavel Machek
2019-06-11 21:58 ` Matthias Kaehlcke [this message]
2019-06-11 21:58 ` Matthias Kaehlcke
2019-06-11 15:33 ` Daniel Thompson
2019-06-11 15:33 ` Daniel Thompson
2019-06-11 15:33 ` Daniel Thompson
2019-06-11 17:01 ` Matthias Kaehlcke
2019-06-11 17:01 ` Matthias Kaehlcke
2019-06-11 10:14 ` [PATCH 1/2] dt-bindings: pwm-backlight: Add 'max-brightness' property Pavel Machek
2019-06-11 10:14 ` Pavel Machek
2019-06-11 10:28 ` Thierry Reding
2019-06-11 10:28 ` Thierry Reding
2019-06-11 21:38 ` Matthias Kaehlcke
2019-06-11 21:38 ` Matthias Kaehlcke
2019-06-11 20:02 ` Jacek Anaszewski
2019-06-11 20:02 ` Jacek Anaszewski
2019-06-11 22:11 ` Matthias Kaehlcke
2019-06-11 22:11 ` Matthias Kaehlcke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190611215859.GF137143@google.com \
--to=mka@chromium.org \
--cc=b.zolnierkie@samsung.com \
--cc=briannorris@chromium.org \
--cc=daniel.thompson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=enric.balletbo@collabora.com \
--cc=jacek.anaszewski@gmail.com \
--cc=jingoohan1@gmail.com \
--cc=lee.jones@linaro.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pavel@ucw.cz \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.