Re: [PATCH] drm/i915/backlight: Fix VESA backlight possible check condition

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 4/1/2026 9:13 AM, Suraj Kandpal wrote:
> VESA backlight is possible through AUX only when
> BACKLIGHT_AUX_ENABLE_CAPABLE is true or when we use BL_ENABLE bit

This statement seems a bit wrong to me.

 From my understanding of Section 12.2, Display Backlight Control Using 
DPCD Registers (quoting part of spec below):

Backlight Enable may be implemented through the use of an AUX command or 
the BL_ENABLE eDP connector pin.

Backlight Enable control capability is indicated as follows:

  - Through an AUX command : BACKLIGHT_AUX_ENABLE_CAPABLE bit in the 
EDP_GENERAL_CAPABILITY_1 register is set (DPCD 00701h[2] = 1)
  - From the BL_ENABLE eDP connector pin : BACKLIGHT_PIN_ENABLE_CAPABLE 
bit in the EDP_GENERAL_CAPABILITY_1 register is set (DPCD 00701h[1] = 1)

So, backlight enable is possible in two ways: AUX-based or via the eDP 
BL_ENABLE pin.

Similarly, backlight brightness adjustment can be done via AUX-based 
control or PWM pin-based control.
It means there can be three configurations:

1) Full AUX-based: Enable and adjustment both via AUX. We currently 
support this
(apart from the AUX luminance-based backlight control).

2) Hybrid: Enable via the BL_ENABLE pin, adjustment via either AUX or PWM.

3) Fully PWM pin-based: Enable via the BL_ENABLE pin, adjustment via PWM.

(As far as I understand, AUX-based enable and brightness adjustment via 
PWM is not possible.)

So, the first statement needs to be corrected slightly, as it implies 
that the BL_ENABLE bit is part of AUX‑based backlight control, which is 
not the case.

Also, I think this can be added as a note before the call to 
intel_dp_aux_supports_vesa_backlight(), something like:

Currently, VESA backlight is supported only via the full AUX-based 
mechanism (or via the AUX luminance-based backlight control).

#TODO: Implement eDP BL_ENABLE pin-based backlight enable and hybrid mode.

> to enable backlight. Since that is not implemented we need to make
> sure we do not try to manipulate backlight when
> BACKLIGHT_AUX_ENABLE_CAPABLE is not set.
> Also fix return value when condition is not fulfilled.
>
> Fixes: 40d2f5820951 ("drm/i915/backlight: Remove try_vesa_interface")


I think fixes should have this commit :

0fb03890d182 ("drm/i915/backlight: Check if VESA backlight is possible")


> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
>   1 file changed, 3 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 d0c76632a946..138d5b5e5482 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -615,8 +615,9 @@ check_if_vesa_backlight_possible(struct intel_dp *intel_dp)
>   	int ret;
>   	u8 bit_min, bit_max;
>   
> -	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP))
> -		return true;
> +	if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP &&
> +	      intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP))
> +		return false;


Hmm... we already validate DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP in 
drm_edp_backlight_supported(), which establishes that the TCON supports 
backlight adjustment.

At this point, for AUX‑only VESA backlight, we should explicitly check:

DP_EDP_BACKLIGHT_AUX_ENABLE_CAP (00701h[2]) : backlight can be enabled 
via AUX
DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP (00702h[1]) : brightness can be 
set via AUX

So, I do agree that both checks are needed, but IMHO we should check 
Enable capability first and then the Adjustment capability.


Regards,
Ankit

>   
>   	ret = drm_dp_dpcd_read_byte(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &bit_min);
>   	if (ret < 0)