Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 23 Jun 2023 12:32:07 +0100
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
Message-ID: <20230623113207.GB4426@aspen.lan>
References: <20230621215457.11297-1-alex@shruggie.ro>
 <20230622021303.GA68330-robh@kernel.org>
 <CAH3L5Qp3q=K5w+LbccZBJqvkz98WgFLqg__y7Be_=-2GsWQs+Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAH3L5Qp3q=K5w+LbccZBJqvkz98WgFLqg__y7Be_=-2GsWQs+Q@mail.gmail.com>
Precedence: bulk

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.