[PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Suraj Kandpal]

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 <suraj.kandpal@intel.com>
---
 .../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);
+	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) {
-- 
2.34.1

RE: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Kandpal, Suraj]

Forgot to mention the closes tag will update in next revision along with any comments if any

> Subject: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-
> AUX VESA path
> 
> 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.
> 

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16043

Regards,
Suraj Kandpal

> Fixes: 40d2f5820951 ("drm/i915/backlight: Remove try_vesa_interface")
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  .../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);
> +	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) {
> --
> 2.34.1

Re: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Jani Nikula]

On Wed, 13 May 2026, Suraj Kandpal <suraj.kandpal@intel.com> 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 <suraj.kandpal@intel.com>
> ---
>  .../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.

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

RE: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Kandpal, Suraj]

> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in
> full-AUX VESA path
> 
> On Wed, 13 May 2026, Suraj Kandpal <suraj.kandpal@intel.com> 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 <suraj.kandpal@intel.com>
> > ---
> >  .../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

✗ Fi.CI.BUILD: warning for drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Patchwork]

== Series Details ==

Series: drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path
URL   : https://patchwork.freedesktop.org/series/166493/
State : warning

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/166493/revisions/1/mbox/ not found

✓ i915.CI.BAT: success for drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]

== Series Details ==

Series: drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path
URL   : https://patchwork.freedesktop.org/series/166493/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_18481 -> Patchwork_166493v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_166493v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-dg2-8:          [PASS][1] -> [DMESG-FAIL][2] ([i915#12061]) +1 other test dmesg-fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-8/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-9:          [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-9/igt@i915_selftest@live@workarounds.html
    - bat-dg2-14:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-14/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-14/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-6:         [DMESG-FAIL][7] ([i915#12061]) -> [PASS][8] +1 other test pass
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061


Build changes
-------------

  * Linux: CI_DRM_18481 -> Patchwork_166493v1

  CI-20190529: 20190529
  CI_DRM_18481: 896e027423be1efc1b1a690bc56bafe6cf49c213 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8909: e68d82b442e3909dd053c97542aeb029707124cf @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_166493v1: 896e027423be1efc1b1a690bc56bafe6cf49c213 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/index.html

[-- Attachment #2: Type: text/html, Size: 3615 bytes --]

✓ i915.CI.BAT: success for drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]

== Series Details ==

Series: drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path
URL   : https://patchwork.freedesktop.org/series/166493/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_18481 -> Patchwork_166493v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_166493v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-dg2-8:          [PASS][1] -> [DMESG-FAIL][2] ([i915#12061]) +1 other test dmesg-fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-8/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-9:          [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-9/igt@i915_selftest@live@workarounds.html
    - bat-dg2-14:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-dg2-14/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-dg2-14/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-6:         [DMESG-FAIL][7] ([i915#12061]) -> [PASS][8] +1 other test pass
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18481/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061


Build changes
-------------

  * Linux: CI_DRM_18481 -> Patchwork_166493v1

  CI-20190529: 20190529
  CI_DRM_18481: 896e027423be1efc1b1a690bc56bafe6cf49c213 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8909: e68d82b442e3909dd053c97542aeb029707124cf @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_166493v1: 896e027423be1efc1b1a690bc56bafe6cf49c213 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166493v1/index.html

[-- Attachment #2: Type: text/html, Size: 3615 bytes --]

RE: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in full-AUX VESA path

[Jani Nikula]

On Wed, 13 May 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> <arun.r.murthy@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>
>> Subject: Re: [PATCH] drm/i915/backlight: Sanitize BIOS-enabled PCH PWM in
>> full-AUX VESA path
>> 
>> On Wed, 13 May 2026, Suraj Kandpal <suraj.kandpal@intel.com> 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 <suraj.kandpal@intel.com>
>> > ---
>> >  .../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