On Thu, Oct 17, 2019 at 7:19 AM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>
> On Thu, Oct 17, 2019 at 12:47:27PM +0100, Daniel Thompson wrote:
> > On Thu, Oct 17, 2019 at 10:10:59AM +0200, Uwe Kleine-König wrote:
> > > A previous change in the pwm core (namely 01ccf903edd6 ("pwm: Let
> > > pwm_get_state() return the last implemented state")) changed the
> > > semantic of pwm_get_state() and disclosed an (as it seems) common
> > > problem in lowlevel PWM drivers. By not relying on the period and duty
> > > cycle being retrievable from a disabled PWM this type of problem is
> > > worked around.
> > >
> > > Apart from this issue only calling the pwm_get_state/pwm_apply_state
> > > combo once is also more effective.
> >
> > I'm only interested in the second paragraph here.
> >
> > There seems to be a reasonable consensus that the i.MX27 and cros-ec
> > PWM drivers should be fixed for the benefit of other PWM clients.
> > So we make this change because it makes the pwm-bl better... not to
> > work around bugs ;-).
>
> That's fine, still I think it's fair to explain the motivation of
> creating this patch.
>
> > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> > > index 746eebc411df..ddebd62b3978 100644
> > > --- a/drivers/video/backlight/pwm_bl.c
> > > +++ b/drivers/video/backlight/pwm_bl.c
> > > @@ -67,40 +62,27 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb)
> > >
> > > static void pwm_backlight_power_off(struct pwm_bl_data *pb)
> > > {
> > > - struct pwm_state state;
> > > -
> > > - pwm_get_state(pb->pwm, &state);
> > > - if (!pb->enabled)
> > > - return;
> > > -
> >
> > Why remove the pb->enabled check? I thought that was there to ensure we
> > don't mess up the regular reference counts.
>
> I havn't looked yet, but I guess I have to respin. Expect a v2 later
> today.
I would agree that a high-level fix is better than a series of low
level driver fixes. For what its worth, your V1 patch worked fine on
my i.MX6Q. I can test the V2 patch when its ready.
adam
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
