Re: [PATCH] drm/i915/backlight: Remove try_vesa_interface

[Werner Sembach <wse@tuxedocomputers.com>]

Hi,

Am 24.02.26 um 04:13 schrieb Suraj Kandpal:
> Remove try_vesa_interface. This is because we now make VESA Interface
> as a fallback mechanism for Panels which needs VESA DPCD AUX backlight
> mechanism to work but have a broken VBT indicating otherwise.
> While at in sneak in a small comment cleanup too.

Just a heads up: I ran into some devices that don't auto select the intel 
interface and don't work with the vesa interface, but ran in the return -ENODEV 
case before and then still had a working backlight. This patch stops backlight 
control working on these devices. So there seems to be another fallback that 
this patch overwrites?

Explicitly forcing the intel interface via module property makes the backlight 
control work again.

I will do some more research and then write a proper bug report, but just to let 
you know that this patch will cause a regression bug in its current form.

Best regards,

Werner

>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15679
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   .../drm/i915/display/intel_dp_aux_backlight.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 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 eb05ef4bd9f6..a8e9872566cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -644,9 +644,10 @@ 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, try_vesa_interface = false;
> +	bool try_intel_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) {
> @@ -655,7 +656,6 @@ 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;
> @@ -668,20 +668,12 @@ 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
> @@ -694,6 +686,9 @@ 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 VBT's may advertise false Intel backlight support even if the
> +	 * tcon's DPCD says otherwise. This mean we keep VESA interface as fallback in that case.
>   	 */
>   	if (try_intel_interface && intel_dp_aux_supports_hdr_backlight(connector) &&
>   	    intel_dp->edp_dpcd[0] <= DP_EDP_14b) {
> @@ -703,7 +698,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
>   		return 0;
>   	}
>   
> -	if (try_vesa_interface && intel_dp_aux_supports_vesa_backlight(connector)) {
> +	if (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;