RE: [PATCH v3 1/8] drm/i915/backlight: Use default/max brightness for VESA AUX backlight init

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 02 Mar 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> Subject: RE: [PATCH v3 1/8] drm/i915/backlight: Use default/max brightness for
>> VESA AUX backlight init
>> 
>> On Mon, 02 Mar 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> >> Subject: RE: [PATCH v3 1/8] drm/i915/backlight: Use default/max
>> >> brightness for VESA AUX backlight init
>> >>
>> >> On Wed, 25 Feb 2026, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> >> >> Subject: Re: [PATCH v3 1/8] drm/i915/backlight: Use default/max
>> >> >> brightness for VESA AUX backlight init
>> >> >>
>> >> >> On Tue, 24 Feb 2026, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
>> >> >> > If the brightness fetched from VBT/previous state is 0 on
>> >> >> > backlight initialization, then set the brightness to a default/max value.
>> >> >> > Whenever the minimum brightness is reported as 0 there are
>> >> >> > chances we end up with blank screen. This confuses the user into
>> >> >> > thinking the display is acting weird. This occurs in eDP 1.5
>> >> >> > when we are using PANEL_LUMINANCE_OVERRIDE mode to mainpulate
>> >> >> > brightness via luminance values.
>> >> >> >
>> >> >> > Closes:
>> >> >> > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15671
>> >> >> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> >> >> > Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
>> >> >> > ---
>> >> >> > v1 -> v2:
>> >> >> > - Let users set brightness to 0, make it so that it's just not
>> >> >> > done by default (Arun)
>> >> >> >
>> >> >> > v2 -> v3:
>> >> >> > -Update commit header and message (Arun)
>> >> >> >
>> >> >> >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 4 ++++
>> >> >> >  1 file changed, 4 insertions(+)
>> >> >> >
>> >> >> > 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 eb05ef4bd9f6..c40ce310ad97 100644
>> >> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> >> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> >> >> > @@ -564,6 +564,8 @@ static int
>> >> >> > intel_dp_aux_vesa_setup_backlight(struct
>> >> >> intel_connector *connector,
>> >> >> >  		}
>> >> >> >  		panel->backlight.level =
>> >> >> intel_dp_aux_vesa_get_backlight(connector, 0);
>> >> >> >  		panel->backlight.enabled = panel->backlight.level != 0;
>> >> >> > +		if (!panel->backlight.level)
>> >> >> > +			panel->backlight.level = panel->backlight.max;
>> >> >>
>> >> >> How does this help when .enabled is still based on level != 0 above?
>> >> >>
>> >> >
>> >> > Well we keep the backlight.enabled as false if we read a 0 back
>> >> > from the DPCD
>> >> or the current level state is 0.
>> >> > This is to maintain the policy that if during setup we get 0 as
>> >> > backlight value eDP backlight is currently disabled (which means
>> >> > __intel_backlight_enable needs be called). We then change the
>> >> > current level to max so that when backlight enable is called after
>> >> > setup from
>> >> intel_backlight_update, we enable backlight with max level so that we
>> >> do not end up with a blank screen. This is also where we set
>> backlight.enabled = true.
>> >> > This is  to tackle different eDP behavior where, some preserve the
>> >> > last brightness value programmed in them (in that case users want
>> >> > the same brightness to continue) while others don't and just 0 it
>> >> > out instead of
>> >> having some default value (in that case we keep backlight.enabled =
>> >> false later to be made true during the __intel_backlight_enable call).
>> >> > We face these scenarios in some compositors during the pass key
>> >> > phase where the compositor is still totally not doing everything
>> >> > and does not send
>> >> us any explicit brightness value to set thinking eDP would have some
>> >> basic default value of it's own . We end up getting a 0 from DPCD and
>> >> we enable and set the backlight enable with 0 value which anyways
>> >> later causes us to call backlight disable.
>> >> > In this case during authentication in some compositors like Fedora
>> >> > there are cases where we do not get a explicitly backlight value
>> >> > till the user
>> >> has to blindly enter their Passkey, after which the compositor sends
>> >> us some sane value which we then program.
>> >>
>> >> There's a long history of problems with the PWM backlight
>> >> unexpectedly going from 0 to max.
>> >
>> > Right but at least with this now luminance values will continue if
>> > DPCD maintains its state if we get a value back, otherwise we set a Default
>> value.
>> 
>> What's the brightness control mode *before* we enable luminance control?
>> 
>> When taking over, we should try to read the current brightness setting with the
>> current brightness control method. If we're switching to luminance control, the
>> existing luminance value is meaningless.
>> 
>> AFAICT drm_edp_backlight_probe_state() uses bl->luminance_set to determine
>> the value to read, not the current mode. At a glance, seems wrong to me.
>
> Luminance mode is the current mode. Which we determine that by checking different capabilities from the and setting them
> In this case aux_set and aux_enable to represent them.

The question is not about the panel's *ability* to use luminance mode,
it's about whether that mode was set and in use by GOP/pre-os.

BR,
Jani.

>
> [    1.667694] i915 0000:00:02.0: [drm:drm_edp_backlight_init [drm_display_helper]] AUX A/DDI A/PHY A: Found backlight: aux_set=1 aux_enable=0 mode=0
> [    1.667703] i915 0000:00:02.0: [drm:drm_edp_backlight_init [drm_display_helper]] AUX A/DDI A/PHY A: Backlight caps: level=496/496 pwm_freq_pre_divider=0 lsb_reg_used=1
>
> In this case aux_set = 1 and luminance_set = 1 which means we are in luminance mode
>
>> 
>> Of course, regressions have priority, so a revert should also be a consideration
>> before quickly going for adding level = max in there.
>> 
>
> From what I can see
> We are in Luminance Mode to begin with. From logs there is a level mentioned in VBT should we use that ?
>
> [    1.665632] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Panel type (VBT): 255
> [    1.665770] i915 0000:00:02.0: [drm:pnpid_get_panel_type [i915]] EDID manufacturer name: SDC, product code: 16899, serial number: 0, year of manufacture: 2024
> [    1.665890] i915 0000:00:02.0: [drm:pnpid_get_panel_type [i915]] EDID raw product id: 4c 83 03 42 00 00 00 00 00 22
> [    1.666006] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Panel type (fallback): 0
> [    1.666124] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Selected panel type (fallback): 0
> [    1.666235] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] DRRS supported mode is seamless
> [    1.666346] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Found panel mode in BIOS VBT legacy lfp table: "640x480": 63 25180 640 648 744 784 480 482 484 509 0x8 0xa
> [    1.666454] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Panel manufacturer name: @H@, product code: 0, serial number: 0, year of manufacture: 1990
> [    1.666560] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Panel name: LFP_PanelName
> [    1.666665] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] Seamless DRRS min refresh rate: 0 Hz
> [    1.666757] i915 0000:00:02.0: [drm:intel_bios_init_panel [i915]] VBT backlight PWM modulation frequency 200 Hz, active high, min brightness 0, level 255, controller 0
> [    1.666847] i915 0000:00:02.0: [drm:intel_panel_add_edid_fixed_modes [i915]] [CONNECTOR:502:eDP-1] using preferred EDID fixed mode: "2880x1800": 60 709633 2880 2888 2920 3080 1800 3800 3816 3840 0x48 0xa
> [    1.666931] i915 0000:00:02.0: [drm:intel_panel_add_edid_fixed_modes [i915]] [CONNECTOR:502:eDP-1] using alternate EDID fixed mode: "2880x1800": 120 709633 2880 2888 2920 3080 1800 1880 1896 1920 0x40 0xa
> [    1.667117] mmc0: SDHCI controller on PCI [0000:58:00.0] using ADMA
> [    1.667206] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]] AUX A/DDI A/PHY A: 0x007a4 AUX -> (ret=  1) 00
> [    1.667223] i915 0000:00:02.0: [drm:intel_dp_aux_init_backlight_funcs [i915]] [CONNECTOR:502:eDP-1] AUX Luminance Based Backlight Control Supported!
> [    1.667335] i915 0000:00:02.0: [drm:intel_dp_aux_init_backlight_funcs [i915]] [CONNECTOR:502:eDP-1] Using VESA eDP backlight controls
> [    1.667413] i915 0000:00:02.0: [drm:intel_panel_init [i915]] [CONNECTOR:502:eDP-1] DRRS type: none
>
> VBT here says use level 255 would it be okay if we set that to level as VBT level, if no value is returned from DPCD panel.
>
> Regards,
> Suraj Kandpal
>
>> > Can we proceed with getting this merged ? Would really help the user.
>> 
>> The real problem with quick fixes to help the user is that they have the
>> potential to make it a lot harder for a lot more users and developers in the long
>> run.
>> 
>> 
>> BR,
>> Jani.
>> 
>> >
>> > Regards,
>> > Suraj Kandpal
>> >
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >> >
>> >> > Regards,
>> >> > Suraj Kandpal
>> >> >
>> >> >> >  		drm_dbg_kms(display->drm,
>> >> >> >  			    "[CONNECTOR:%d:%s] AUX VESA Nits
>> backlight level
>> >> >> is controlled through DPCD\n",
>> >> >> >  			    connector->base.base.id, connector-
>> >base.name);
>> >> >> @@ -573,6
>> >> >> > +575,8 @@ static int intel_dp_aux_vesa_setup_backlight(struct
>> >> >> intel_connector *connector,
>> >> >> >  		if (current_mode ==
>> >> >> DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
>> >> >> >  			panel->backlight.level = current_level;
>> >> >> >  			panel->backlight.enabled = panel-
>> >backlight.level != 0;
>> >> >> > +			if (!panel->backlight.level)
>> >> >> > +				panel->backlight.level = panel-
>> >backlight.max;
>> >> >>
>> >> >> Ditto.
>> >> >>
>> >> >> >  		} else {
>> >> >> >  			panel->backlight.level = panel->backlight.max;
>> >> >> >  			panel->backlight.enabled = false;
>> >> >>
>> >> >> --
>> >> >> Jani Nikula, Intel
>> >>
>> >> --
>> >> Jani Nikula, Intel
>> 
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel