| Message ID | 20211121110032.4720-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/backlight: Drop duplicate intel_panel_actually_set_backlight() | expand |
On Sun, 21 Nov 2021, Hans de Goede <hdegoede@redhat.com> wrote: > At least the Bay Trail LPSS PWM controller used with DSI panels on many > Bay Trail tablets seems to leave the PWM pin in whatever state it was > (high or low) ATM that the PWM gets disabled. Combined with some panels > not having a separate backlight-enable pin this leads to the backlight > sometimes staying on while it should not (when the pin was high during > PWM-disabling). > > First calling intel_backlight_set_pwm_level() will ensure that the pin > is always low (or high for inverted brightness panels) since the passed > in duty-cycle is 0% (or 100%) when the PWM gets disabled fixing the > backlight sometimes staying on. > > With the exception of ext_pwm_disable_backlight() all other > foo_disable_backlight() functions call intel_backlight_set_pwm_level() > already before disabling the backlight, so this change also aligns > ext_pwm_disable_backlight() with all the other disable() functions. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> I'll take your word for it. Acked-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_backlight.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c > index 03cd730c926a..2758a2f6c093 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_backlight.c > @@ -421,6 +421,8 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn > struct intel_connector *connector = to_intel_connector(old_conn_state->connector); > struct intel_panel *panel = &connector->panel; > > + intel_backlight_set_pwm_level(old_conn_state, level); > + > panel->backlight.pwm_state.enabled = false; > pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); > }
Reviewed-by: Lyude Paul <lyude@redhat.com> On Sun, 2021-11-21 at 12:00 +0100, Hans de Goede wrote: > At least the Bay Trail LPSS PWM controller used with DSI panels on many > Bay Trail tablets seems to leave the PWM pin in whatever state it was > (high or low) ATM that the PWM gets disabled. Combined with some panels > not having a separate backlight-enable pin this leads to the backlight > sometimes staying on while it should not (when the pin was high during > PWM-disabling). > > First calling intel_backlight_set_pwm_level() will ensure that the pin > is always low (or high for inverted brightness panels) since the passed > in duty-cycle is 0% (or 100%) when the PWM gets disabled fixing the > backlight sometimes staying on. > > With the exception of ext_pwm_disable_backlight() all other > foo_disable_backlight() functions call intel_backlight_set_pwm_level() > already before disabling the backlight, so this change also aligns > ext_pwm_disable_backlight() with all the other disable() functions. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/gpu/drm/i915/display/intel_backlight.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c > b/drivers/gpu/drm/i915/display/intel_backlight.c > index 03cd730c926a..2758a2f6c093 100644 > --- a/drivers/gpu/drm/i915/display/intel_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_backlight.c > @@ -421,6 +421,8 @@ static void ext_pwm_disable_backlight(const struct > drm_connector_state *old_conn > struct intel_connector *connector = > to_intel_connector(old_conn_state->connector); > struct intel_panel *panel = &connector->panel; > > + intel_backlight_set_pwm_level(old_conn_state, level); > + > panel->backlight.pwm_state.enabled = false; > pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); > }
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 03cd730c926a..2758a2f6c093 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -421,6 +421,8 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct intel_panel *panel = &connector->panel; + intel_backlight_set_pwm_level(old_conn_state, level); + panel->backlight.pwm_state.enabled = false; pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); }
At least the Bay Trail LPSS PWM controller used with DSI panels on many Bay Trail tablets seems to leave the PWM pin in whatever state it was (high or low) ATM that the PWM gets disabled. Combined with some panels not having a separate backlight-enable pin this leads to the backlight sometimes staying on while it should not (when the pin was high during PWM-disabling). First calling intel_backlight_set_pwm_level() will ensure that the pin is always low (or high for inverted brightness panels) since the passed in duty-cycle is 0% (or 100%) when the PWM gets disabled fixing the backlight sometimes staying on. With the exception of ext_pwm_disable_backlight() all other foo_disable_backlight() functions call intel_backlight_set_pwm_level() already before disabling the backlight, so this change also aligns ext_pwm_disable_backlight() with all the other disable() functions. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/display/intel_backlight.c | 2 ++ 1 file changed, 2 insertions(+)