On Mon, 02 Mar 2026, "Kandpal, Suraj" <[email protected]> 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" <[email protected]> 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" <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> >> >> > Reviewed-by: Arun R Murthy <[email protected]>
>> >> >> > ---
>> >> >> > 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
