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-fixes
>> 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%[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/[email protected]/
> 
> 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?

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;
>>
>