Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Mon, 23 Oct 2023 13:49:09 +0100
From:   Daniel Thompson <daniel.thompson@linaro.org>
To:     Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>
Cc:     Philipp Zabel <p.zabel@pengutronix.de>,
        Thierry Reding <thierry.reding@gmail.com>,
        Lee Jones <lee@kernel.org>, Jingoo Han <jingoohan1@gmail.com>,
        Helge Deller <deller@gmx.de>, linux-pwm@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH] backlight: pwm_bl: Avoid backlight flicker applying
 initial PWM state
Message-ID: <20231023124909.GC49511@aspen.lan>
References: <20230608-backlight-pwm-avoid-flicker-v1-1-afd380d50174@pengutronix.de>
 <20231018210741.6t3yfj6qgmpwhhlo@pengutronix.de>
 <20231020112727.GF23755@aspen.lan>
 <20231020121148.3g6t3v5uuyubifpb@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231020121148.3g6t3v5uuyubifpb@pengutronix.de>
Precedence: bulk

On Fri, Oct 20, 2023 at 02:11:48PM +0200, Uwe Kleine-K?nig wrote:
> Hello Daniel,
>
> On Fri, Oct 20, 2023 at 12:27:27PM +0100, Daniel Thompson wrote:
> > On Wed, Oct 18, 2023 at 11:07:41PM +0200, Uwe Kleine-K?nig wrote:
> > > Hello Philipp,
> > >
> > > On Thu, Jun 08, 2023 at 04:11:14PM +0200, Philipp Zabel wrote:
> > > > The initial PWM state returned by pwm_init_state() has a duty cycle
> > > > of 0 ns.
> > >
> > > This is only true for drivers without a .get_state() callback, isn't it?
> >
> > pwm_init_state() explicitly zeros the duty-cycle in order to avoid
> > problems when the default args have a different period to the currently
> > applied config:
> > https://elixir.bootlin.com/linux/latest/source/include/linux/pwm.h#L174
>
> Ah right, pwm_init_state() is strange in a different way than I
> remembered :-) pwm_get_state() is only called to get .enabled set
> appropriately.
>
> Looking at the callers:
>
>  <snip>

>  - drivers/video/backlight/lm3630a_bl.c
>    explictily sets .enabled before calling pwm_apply_state()
>
>  - drivers/video/backlight/lp855x_bl.c
>    explictily sets .enabled before calling pwm_apply_state()
>
>  - drivers/video/backlight/pwm_bl.c
>    This is the one we currently discuss. I think even with the patch
>    applied it uses the .enabled value returned by pwm_init_state() but
>    it shouldn't.

Agreed.

> So all consumers using pwm_init_state() either don't use the .enabled
> value returned by pwm_init_state() or at least shouldn't do that.

Looking a little deeper in the PWM code, it looks to me like pwm_bl.c
could just use pwm_adjust_config() during probe to transition between
bootloader settings and kernel settings!


Daniel.