> <[email protected]>; Kandpal, Suraj <[email protected]>
> Subject: Re: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in
> full-AUX VESA path
>
> On Wed, 13 May 2026, Suraj Kandpal <[email protected]> wrote:
> > In full-AUX VESA mode (aux_enable && aux_set) the driver never touches
> > the native PCH PWM. If BIOS left PWM CTL register enabled, the PCH PWM
> > keeps system alive during s2idle and blocks S0ix.
> > Always run pwm_funcs->setup() so pwm_enabled reflects real HW state,
> > and on first enable in full-AUX mode call pwm_funcs->disable() once to
> > clear the stale bit. Runtime behaviour is otherwise unchanged.
> >
> > Fixes: 40d2f5820951 ("drm/i915/backlight: Remove try_vesa_interface")
> > Signed-off-by: Suraj Kandpal <[email protected]>
> > ---
> > .../drm/i915/display/intel_dp_aux_backlight.c | 32
> > +++++++++++++------
> > 1 file changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > index a8d56ebf06a2..c828c568fb8b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -496,6 +496,17 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> > struct intel_panel *panel = &connector->panel;
> > struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> >
> > + /*
> > + * In full AUX VESA mode the native PWM is never driven by us. If BIOS
> > + * left it enabled, the PCH PWM keeps the system alive and blocks
> > + * S0ix. Sanitize it once via pwm_funcs->disable.
> > + */
> > + if (panel->backlight.edp.vesa.info.aux_enable &&
> > + panel->backlight.edp.vesa.info.aux_set &&
> > + panel->backlight.pwm_enabled)
> > + panel->backlight.pwm_funcs->disable(conn_state,
> > +
> intel_backlight_invert_pwm_level(connector, 0));
> > +
> > if (!(panel->backlight.edp.vesa.info.aux_enable ||
> > panel->backlight.edp.vesa.info.luminance_set)) {
> > u32 pwm_level;
> > @@ -558,15 +569,18 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
> > panel-
> >backlight.edp.vesa.info.luminance_set),
> > backlight_unit_str(panel));
> >
> > - if (!panel->backlight.edp.vesa.info.aux_set ||
> > - !panel->backlight.edp.vesa.info.aux_enable) {
> > - ret = panel->backlight.pwm_funcs->setup(connector, pipe);
> > - if (ret < 0) {
> > - drm_err(display->drm,
> > - "[CONNECTOR:%d:%s] Failed to setup PWM
> backlight controls for eDP backlight: %d\n",
> > - connector->base.base.id, connector-
> >base.name, ret);
> > - return ret;
> > - }
> > + /*
> > + * Always probe the native PWM HW state so panel-
> >backlight.pwm_enabled
> > + * reflects what BIOS left behind. Required for the full-AUX VESA path
> > + * to detect and sanitize a BIOS-enabled PCH PWM that would
> otherwise
> > + * block S0ix.
> > + */
> > + ret = panel->backlight.pwm_funcs->setup(connector, pipe);
>
> This will log something like "Using native PWM for backlight control" in
> dmesg,
> which is going to be wildly confusing for AUX backlight.
Yes true I was wondering the same thing.
I was thinking is changing this message to Setting up PWM function for
backlight makes more sense.
Or another option is to just
Have the aux_enable && aux_set check inside these PWM functions to skip this
print al together.
We do need this setup to fill up all the pwm related fields inside backlight.
Moreover this helps us call pwm_funcs->disable which take care of choosing the
correct gen of backlight register to disable.
Which do you think makes more sense Jani.
Regards,
Suraj Kandpal
>
> BR,
> Jani.
>
> > + if (ret < 0) {
> > + drm_err(display->drm,
> > + "[CONNECTOR:%d:%s] Failed to setup PWM backlight
> controls for eDP backlight: %d\n",
> > + connector->base.base.id, connector->base.name, ret);
> > + return ret;
> > }
> >
> > if (panel->backlight.edp.vesa.info.luminance_set) {
>
> --
> Jani Nikula, Intel
