Re: [PATCH] backlight: pwm_bl: Avoid backlight flicker if backlight control GPIO is input

From: Daniel Thompson <daniel.thompson@linaro.org>
To: Marek Vasut <marex@denx.de>
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	Christian Gmeiner <christian.gmeiner@gmail.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Thierry Reding <treding@nvidia.com>,
	linux-pwm@vger.kernel.org
Subject: Re: [PATCH] backlight: pwm_bl: Avoid backlight flicker if backlight control GPIO is input
Date: Fri, 23 Jul 2021 12:17:24 +0100	[thread overview]
Message-ID: <20210723111724.q4yu2ocgn5fdzge6@maple.lan> (raw)
In-Reply-To: <20210723101510.r2xz4rlzvgkhxtw3@maple.lan>

On Fri, Jul 23, 2021 at 11:15:10AM +0100, Daniel Thompson wrote:
> On Thu, Jul 22, 2021 at 09:02:04PM +0200, Marek Vasut wrote:
> > On 7/22/21 1:28 PM, Daniel Thompson wrote:
> > > On Wed, Jul 21, 2021 at 08:46:42PM +0200, Marek Vasut wrote:
> > > > On 7/21/21 6:12 PM, Daniel Thompson wrote:
> > > > > On Wed, Jul 21, 2021 at 05:09:57PM +0200, Marek Vasut wrote:
> > > > > > On 7/21/21 12:49 PM, Daniel Thompson wrote:
> > > > > [...]
> > > > > This sails very close to the
> > > > > edge of what is in-scope for DT (at least it does it we can read
> > > > > the inherited state directly from the hardware).
> > > > 
> > > > The problem with reading it out of hardware is that the hardware might be in
> > > > undefined state and expects Linux to define that state, so that does not
> > > > always work. Hence my initial suggestion to add a DT property to define the
> > > > state up front, instead of using these fragile heuristics.
> > > 
> > > To achieve a flicker-free boot we must know the initial state of the
> > > backlight (not just the enable pin).
> > 
> > The backlight hardware might be in uninitialized state and then Linux should
> > set the state, likely based on something in DT, because there is no previous
> > state to read.
> 
> There is always a previous state. The kernel doesn't care whether that
> previous state was imposed by a power-on reset, the bootloader or a
> kexec.
> 
> For the driver to come up flicker-free in all the different cases we
> need to know whether the backlight is currently emitting light or not
> and, if it is emitting light, then we need to know what the duty cycle
> is (currently we inherit require the PWM driver to correctly inherit the
> duty cycle from the hardware).

Oops... this is wrong (I think it is cross-talk from an old patch). We
do not currently inherit the duty cycle.

> So far, the previous state has been observable by the lower level
> drivers (GPIO, PWM, regulator). I remain reluctant to provide
> workarounds for cases where it is not observable without motivating
> hardware. I certainly wouldn't want to make such bindings mandatory
> since observable hardware registers are a far more reliable source of
> truth than what the DT tells us about what it thinks the bootloader
> (or power-on reset) actually did ;-).

Which makes conclusion badly reasoned.

However, until we can clearly articulate whether the problem we want to
solve is describing the initial backlight state or specifying the default
(post-probe) power state for the legacy cases I'm still content not to
change things ;-).

> > > [...]
> > > Wow! That is *way* longer than I intended when I started writing it.
> > > Anyhow I suspect any disconnect comes about due to the difference in
> > > backlight state *after* probe being, in part, to software structure
> > > rather than purely a hardware property.
> > 
> > Maybe this should be added to documentation.
> 
> I'll see what I can do.

Done, see v3. I think it is better explained than the e-mail version.

Daniel.