On Wed, 13 May 2026, "Kandpal, Suraj" <[email protected]> wrote:
>> <[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.
I guess s/Using/Setting up/ works.
Maybe intel_pwm_setup_backlight() needs to say something about
"Using". Then it would align with what intel_dp_aux_backlight.c says
about "Using".
> Or another option is to just
> Have the aux_enable && aux_set check inside these PWM functions to skip this
> print al together.
Yeah, no. Need to try to maintain division of responsibilities, and
avoid touching data that belongs to the other part.
BR,
Jani.
> 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
--
Jani Nikula, Intel
