RE: [PATCH] Revert "drm/i915/backlight: Remove try_vesa_interface"

> -----Original Message-----
> From: Tvrtko Ursulin <[email protected]>
> Sent: Friday, June 5, 2026 3:23 PM
> To: Thorsten Leemhuis <[email protected]>; Tvrtko Ursulin
> <[email protected]>; Dave Airlie <[email protected]>; Simona Vetter
> <[email protected]>; Kandpal, Suraj <[email protected]>
> Cc: Nautiyal, Ankit K <[email protected]>; Murthy, Arun R
> <[email protected]>; [email protected]; intel-
> [email protected]; Vivi, Rodrigo <[email protected]>; Linux kernel
> regressions list <[email protected]>; ML dri-devel <dri-
> [email protected]>; Jani Nikula <[email protected]>
> Subject: Re: [PATCH] Revert "drm/i915/backlight: Remove try_vesa_interface"
> 
> 
> Hi Suraj,
> 
> On 05/06/2026 07:58, Thorsten Leemhuis wrote:
> > On 6/4/26 15:55, Thorsten Leemhuis wrote:
> >> On 5/17/26 04:47, Suraj Kandpal wrote:
> >>> This reverts commit 40d2f5820951dee818d05c14677277048bd85f9f.
> >>>
> >>> Removing the try_vesa_interface gate caused a backlight regression
> >>> on panels whose VBT correctly reports INTEL_BACKLIGHT_DISPLAY_DDI
> >>> and whose PWM path is the actual backlight control, but whose DPCD
> >>> optimistically advertises DP_EDP_BACKLIGHT_AUX_ENABLE_CAP /
> _BRIGHTNESS_AUX_SET_CAP.
> >>> After the commit such panels silently bind to the VESA AUX backlight
> >>> funcs; AUX writes complete but the panel ignores them, leaving
> >>> brightness stuck (no-op backlight). Observed on at least KBL and TGL
> >>> eDP setups.
> >>
> >> Lo! What's the status of this regression fix? It's a -next for two
> >> weeks now as f30fddb4402313 ("Revert "drm/i915/backlight: Remove
> >> try_vesa_interface""), but from the outside and checking
> >> https://gitlab.freedesktop.org/drm/i915/kernel/-/commits/drm-intel-fi
> >> xes it looks like it's scheduled for merging in the next cycle.
> >
> > Resending to Tvrtko, who sent the i915 PR yesterday (which didn't
> > contain that fix), as well as Dave and Simona.
> >
> > FWIW, due to the lack of response to various inquiries I'm considering
> > to ask Linus to directly pick up the mentioned regression fix to
> > ensure it makes it into rc7.
> >
> > In case anyone wonder what regression I'm talking about:
> >
> > * https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/16015 /
> > https://lore.kernel.org/lkml/CADo9pHjr-
> zZ9C3%2B026y5%2BXOGPSeRzSJMCHof
> > [email protected]/
> > * https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/16043
> > * https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16097 /
> > https://lore.kernel.org/lkml/d2de7933-e650-4b19-8d88-90d66693dcfc@mess
> > age-id.googlemail.com/
> >
> > Ciao, Thorsten
> >
> >> But I think it should be merged this cycle (ideally before -rc7, as
> >> Linus wants all known regression fixed by -rc6), as it fixes a
> >> regression that is known since the -rc1 days. I already asked for the
> >> mainlining plans in gitlab tickets about a week ago (and since then
> >> affected users spoke up, too), but there was no conclusive answer for
> >> the plans, which is why I'm trying this way now.
> 
> Was there a reason the revert was not marked with a Fixes: tag? Or in other
> words, any particular reason why it should *not* be picked up for drm-intel-
> fixes?

There wasn't any particular reason.
Tbh I didn't think we would require it since this revert was fixing the issue caused by the patch in question being reverted.

Regards,
Suraj Kandpal

> 
> Regards,
> 
> Tvrtko
> 
> >>
> >> Ciao, Thorsten
> >>
> >>
> >>> Signed-off-by: Suraj Kandpal <[email protected]>
> >>> ---
> >>>   .../drm/i915/display/intel_dp_aux_backlight.c | 19 ++++++++++++-------
> >>>   1 file changed, 12 insertions(+), 7 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..7a6c07f6aaeb 100644
> >>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> >>> @@ -691,10 +691,9 @@ int intel_dp_aux_init_backlight_funcs(struct
> intel_connector *connector)
> >>>   	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >>>   	struct drm_device *dev = connector->base.dev;
> >>>   	struct intel_panel *panel = &connector->panel;
> >>> -	bool try_intel_interface = false;
> >>> +	bool try_intel_interface = false, try_vesa_interface = false;
> >>>
> >>> -	/*
> >>> -	 * Check the VBT and user's module parameters to figure out which
> >>> +	/* Check the VBT and user's module parameters to figure out which
> >>>   	 * interfaces to probe
> >>>   	 */
> >>>   	switch (display->params.enable_dpcd_backlight) { @@ -703,6 +702,7
> >>> @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector
> *connector)
> >>>   	case INTEL_DP_AUX_BACKLIGHT_AUTO:
> >>>   		switch (panel->vbt.backlight.type) {
> >>>   		case INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE:
> >>> +			try_vesa_interface = true;
> >>>   			break;
> >>>   		case INTEL_BACKLIGHT_DISPLAY_DDI:
> >>>   			try_intel_interface = true;
> >>> @@ -715,12 +715,20 @@ int intel_dp_aux_init_backlight_funcs(struct
> intel_connector *connector)
> >>>   		if (panel->vbt.backlight.type !=
> INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE)
> >>>   			try_intel_interface = true;
> >>>
> >>> +		try_vesa_interface = true;
> >>> +		break;
> >>> +	case INTEL_DP_AUX_BACKLIGHT_FORCE_VESA:
> >>> +		try_vesa_interface = true;
> >>>   		break;
> >>>   	case INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL:
> >>>   		try_intel_interface = true;
> >>>   		break;
> >>>   	}
> >>>
> >>> +	/* For eDP 1.5 and above we are supposed to use VESA interface for
> brightness control */
> >>> +	if (intel_dp->edp_dpcd[0] >= DP_EDP_15)
> >>> +		try_vesa_interface = true;
> >>> +
> >>>   	/*
> >>>   	 * Since Intel has their own backlight control interface, the majority of
> machines out there
> >>>   	 * using DPCD backlight controls with Intel GPUs will be using
> >>> this interface as opposed to @@ -733,9 +741,6 @@ int
> intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
> >>>   	 * panel with Intel's OUI - which is also required for us to be able to
> detect Intel's
> >>>   	 * backlight interface at all. This means that the only sensible way for us
> to detect both
> >>>   	 * interfaces is to probe for Intel's first, and VESA's second.
> >>> -	 *
> >>> -	 * Also there is a chance some VBTs may advertise false Intel backlight
> support even if the
> >>> -	 * TCON DPCD says otherwise. This means we keep VESA interface as
> fallback in that case.
> >>>   	 */
> >>>   	if (try_intel_interface && intel_dp->edp_dpcd[0] <= DP_EDP_14b &&
> >>>   	    intel_dp_aux_supports_hdr_backlight(connector)) { @@ -745,7
> >>> +750,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector
> *connector)
> >>>   		return 0;
> >>>   	}
> >>>
> >>> -	if (intel_dp_aux_supports_vesa_backlight(connector)) {
> >>> +	if (try_vesa_interface &&
> >>> +intel_dp_aux_supports_vesa_backlight(connector)) {
> >>>   		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Using VESA eDP
> backlight controls\n",
> >>>   			    connector->base.base.id, connector->base.name);
> >>>   		panel->backlight.funcs = &intel_dp_vesa_bl_funcs;
> >>
> >