From: Daniel Thompson <daniel.thompson@linaro.org>
To: Alexandru Ardelean <alex@shruggie.ro>
Cc: Rob Herring <robh@kernel.org>,
dri-devel@lists.freedesktop.org, linux-leds@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, lee@kernel.org,
jingoohan1@gmail.com, pavel@ucw.cz,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
deller@gmx.de, Yannick Fertre <yannick.fertre@foss.st.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: backlight: document new property default-brightness-level
Date: Fri, 23 Jun 2023 12:32:07 +0100 [thread overview]
Message-ID: <20230623113207.GB4426@aspen.lan> (raw)
In-Reply-To: <CAH3L5Qp3q=K5w+LbccZBJqvkz98WgFLqg__y7Be_=-2GsWQs+Q@mail.gmail.com>
On Fri, Jun 23, 2023 at 10:19:57AM +0300, Alexandru Ardelean wrote:
> On Thu, Jun 22, 2023 at 5:13 AM Rob Herring <robh@kernel.org> wrote:
> > > +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> > > default-on:
> > > - description: enable the backlight at boot.
> > > + description:
> > > + The default power state of the backlight at boot.
> > > type: boolean
> > >
> > > + default-brightness-level:
> > > + description:
> > > + The default brightness level on device init. The value can be 0 or 1.
> > > + If omitted, the value is 1. In the context of the "gpio-backlight" driver
> > > + the effect of this setting will be that the backlight is on/off.
> > > + The difference between this setting and "default-on" is that this handles
> > > + brightness, while "default-on" handles the power setting of the device.
> >
> > What power setting? You only have 1 GPIO to control here which is 2
> > states.
There are at least three states: On/Off/HiZ .
Currently the DT description isn't acually rich enough to allow drivers
to safely use the HiZ state so that is not why this change is potentially
useful today (but does illustrate why it is not "wrong" to put it on the
h/ware description).
> > I fail to see why you need 6 possible states with all the
> > combinations of 2 properties.
>
> So, the "default-on" bool gets converted to backlight power settings,
> which eventually gets converted back to GPIO values (at some point).
> Which sounds quirky (when saying/writing it).
Modern DT practice is to for the display to link to backlight. This
gives display control over power state (so backlight automatically
follows the display power state). On such systems the backlight will
be turned "on" when the display hardware comes up (regardless of whether
or not default-on is set).
Thus this control covers the case where we have a display that is
readable when the GPIO is off (e.g. transflexive LCD or epaper).
A display that is readable with the GPIO off means the default
brightness brightness at boot can meaningfully be zero. In this
case the backlight is nominally on but the GPIO is off.
In short, this becomes part of the hardware description, rather than
merely being a driver feature, due to the effect of linking display
to backlight in the DT.
Note also that most backlights do expose on/off via DT for the same
reasons (when the off and zero states both result in the backlight
output pin doing physically the same thing).
> But, yeah.
> That's one thing that also made me a bit undecided to send this.
> On the one hand I like the uniformity it brings.
> On the other hand, because there is the legacy behavior (the
> "default-on" property, and the fact that we can use the GPIO DT
> settings to control this) just explodes complexity/quirks.
>
> We can probably just drop this.
> I'll also admit that my doc-writing skills aren't too great.
It may be potentially useful for people building kit with sunlight
readable displays and trivial backlights as a backup in the dark.
Of course if the pin the backlight is connected to is PWM capable
then the PWM backlight is probably a better bet ;-) .
Daniel.
prev parent reply other threads:[~2023-06-23 11:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-21 21:54 [PATCH v2 1/2] dt-bindings: backlight: document new property default-brightness-level Alexandru Ardelean
2023-06-21 21:54 ` [PATCH v2 2/2] backlight: gpio_backlight: add " Alexandru Ardelean
2023-06-22 2:13 ` [PATCH v2 1/2] dt-bindings: backlight: document " Rob Herring
2023-06-23 7:19 ` Alexandru Ardelean
2023-06-23 11:32 ` Daniel Thompson [this message]
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=20230623113207.GB4426@aspen.lan \
--to=daniel.thompson@linaro.org \
--cc=alex@shruggie.ro \
--cc=conor+dt@kernel.org \
--cc=deller@gmx.de \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jingoohan1@gmail.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lee@kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=robh@kernel.org \
--cc=yannick.fertre@foss.st.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 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).