> -----Original Message----- > From: Jani Nikula <[email protected]> > Sent: Wednesday, June 17, 2026 5:46 PM > To: Murthy, Arun R <[email protected]>; [email protected]; > [email protected] > Cc: Murthy, Arun R <[email protected]> > Subject: Re: [PATCH] drm/i915/backlight: Set brightness to 0 on disable > > On Tue, 16 Jun 2026, Arun R Murthy <[email protected]> wrote: > > On backlight disable for AUX based panels set the brightness to 0 > > before disabling the backlight. PWM based backlight also does set the > > brightness to 0 before disable. > > Both the commit message and the code comments have a lot of the *what* but > are very thin on the *why*. > > Even if there's *zero* explanation of the *what*, it can be deduced from the > code changes. But nothing can replace the *why*. > > Answering the *why* is the single most important thing a commit message > *must* do. > In the same i915 driver PWM based backlight on disable is setting the brightness to '0' before disabling. To maintain unity DPCD based backlight brightness also will have to be set to '0' before disabling. Another reason is Xorg upon receiving shutdown, will kill the graphics clients and then finally the server. Now while killing the clients a particular framebuffer associated with the client will be killed and if there are multiple clients displaying on multiple plane, then one of the plane having fb while the others being shutdown/killed tend to look like a corruption/garbage at the time of backlight being disabled on QHD panels since only the pwm based brightness is set to '0' and not the DPCD brightness. So a momentary corruption just before backlight disable can be seen. Hence while setting PWM based brightness to '0' on shutdown set the DPCD/AUX based brightness to '0' as well.
Thanks and Regards, Arun R Murthy -------------------- > BR, > Jani. > > > > > > Signed-off-by: Arun R Murthy <[email protected]> > > --- > > .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 15 > > +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 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..e4ce39c1eebb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > @@ -355,9 +355,14 @@ intel_dp_aux_hdr_disable_backlight(const struct > drm_connector_state *conn_state, > > struct intel_connector *connector = to_intel_connector(conn_state- > >connector); > > struct intel_panel *panel = &connector->panel; > > > > - /* Nothing to do for AUX based backlight controls */ > > - if (panel->backlight.edp.intel_cap.sdr_uses_aux) > > + /* > > + * Drive the DPCD brightness to 0 before tearing down the link / power > > + * sequencer so the panel can blank emission gracefully. > > + */ > > + if (panel->backlight.edp.intel_cap.sdr_uses_aux) { > > + intel_dp_aux_hdr_set_aux_backlight(conn_state, 0); > > return; > > + } > > > > /* Note we want the actual pwm_level to be 0, regardless of pwm_min > */ > > panel->backlight.pwm_funcs->disable(conn_state, > > intel_backlight_invert_pwm_level(connector, 0)); @@ -519,6 +524,12 @@ > static void intel_dp_aux_vesa_disable_backlight(const struct > drm_connector_state > > struct intel_panel *panel = &connector->panel; > > struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > > > > + /* > > + * Drive the DPCD brightness register to 0 before clearing BL_ENABLE or > > + * dropping the panel power. > > + */ > > + drm_edp_backlight_set_level(&intel_dp->aux, > > +&panel->backlight.edp.vesa.info, 0); > > + > > drm_edp_backlight_disable(&intel_dp->aux, > > &panel->backlight.edp.vesa.info); > > > > if (!(panel->backlight.edp.vesa.info.aux_enable || > > -- > Jani Nikula, Intel
