Linux-PWM Archive on lore.kernel.org
 help / color / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: "Alexandru Stan" <amstan@chromium.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Jingoo Han" <jingoohan1@gmail.com>,
	"Bartlomiej Zolnierkiewicz" <b.zolnierkie@samsung.com>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Rob Herring" <robh+dt@kernel.org>,
	linux-pwm@vger.kernel.org, linux-fbdev@vger.kernel.org,
	"Douglas Anderson" <dianders@chromium.org>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	"Matthias Kaehlcke" <mka@chromium.org>,
	"Enric Balletbo i Serra" <enric.balletbo@collabora.com>
Subject: Re: [PATCH 2/3] backlight: pwm_bl: Artificially add 0% during interpolation
Date: Thu, 13 Aug 2020 14:45:53 +0100
Message-ID: <20200813134553.2hykfvqjtgr4e2pl@holly.lan> (raw)
In-Reply-To: <20200807082113.GI6419@phenom.ffwll.local>

On Fri, Aug 07, 2020 at 10:21:13AM +0200, daniel@ffwll.ch wrote:
> On Mon, Jul 20, 2020 at 09:25:21PM -0700, Alexandru Stan wrote:
> > Some displays need the low end of the curve cropped in order to make
> > them happy. In that case we still want to have the 0% point, even though
> > anything between 0% and 5%(example) would be skipped.
> > 
> > Signed-off-by: Alexandru Stan <amstan@chromium.org>
> > ---
> > 
> >  drivers/video/backlight/pwm_bl.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > index 5193a72305a2..b24711ddf504 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -349,6 +349,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
> >  			/* Fill in the last point, since no line starts here. */
> >  			table[x2] = y2;
> >  
> > +			/*
> > +			 * If we don't start at 0 yet we're increasing, assume
> > +			 * the dts wanted to crop the low end of the range, so
> > +			 * insert a 0 to provide a display off mode.
> > +			 */
> > +			if (table[0] > 0 && table[0] < table[num_levels - 1])
> > +				table[0] = 0;
> 
> Isn't that what the enable/disable switch in backlights are for? There's
> lots of backligh drivers (mostly the firmware variety) where setting the
> backlight to 0 does not shut it off, it's just the lowest setting.
> 
> But I've not been involved in the details of these discussions.

It's been a long standing complaint that the backlight drivers are not
consistent w.r.t. whether 0 means off or lowest. The most commonly used
backlights (ACPI in particular) do not adopt 0 means off but lots of
specific drivers do.

IMHO what is "right" depends on the display technology. For displays
that are essentially black when the backlight is off and become
difficult or impossible to read I'm a little dubious about standardizing
on zero means off. There are situations when zero means off
does make sense however. For example front-lit or transflexive displays
are readable when the "backlight" is off and on these displays it would
make sense.


Daniel.

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-21  4:25 [PATCH 0/3] PWM backlight interpolation adjustments Alexandru Stan
2020-07-21  4:25 ` [PATCH 1/3] backlight: pwm_bl: Fix interpolation Alexandru Stan
2020-09-04 11:27   ` Daniel Thompson
2020-07-21  4:25 ` [PATCH 2/3] backlight: pwm_bl: Artificially add 0% during interpolation Alexandru Stan
2020-08-07  8:21   ` daniel
2020-08-13 13:45     ` Daniel Thompson [this message]
2020-09-04 11:38   ` Daniel Thompson
2020-09-07  7:50     ` Daniel Vetter
2020-09-09 14:45       ` Daniel Thompson
2020-09-09 15:03         ` Daniel Vetter
2020-09-10  7:47           ` Daniel Vetter
2020-09-09 18:42     ` Alexandru M Stan
2020-08-05 21:04 ` [PATCH 0/3] PWM backlight interpolation adjustments Alexandru M Stan

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=20200813134553.2hykfvqjtgr4e2pl@holly.lan \
    --to=daniel.thompson@linaro.org \
    --cc=amstan@chromium.org \
    --cc=b.zolnierkie@samsung.com \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=enric.balletbo@collabora.com \
    --cc=heiko@sntech.de \
    --cc=jingoohan1@gmail.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /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

Linux-PWM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pwm/0 linux-pwm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pwm linux-pwm/ https://lore.kernel.org/linux-pwm \
		linux-pwm@vger.kernel.org
	public-inbox-index linux-pwm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pwm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git