thr3ads.net - Nouveau - [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight [Jun 2025]

If this information is useful, please help other people find it:
Share via:

Suraj Kandpal

2025-Apr-14 04:16 UTC

[PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

Now that drm_edp_backlight init has been to be able to setup
brightness manipulation via luminance we can just use that.

Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
---
 .../drm/i915/display/intel_dp_aux_backlight.c | 100 +++++++++---------
 1 file changed, 49 insertions(+), 51 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 be740fb72ebc..2eff9b545390 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -585,9 +585,36 @@ static int intel_dp_aux_vesa_setup_backlight(struct
intel_connector *connector,
 	u8 current_mode;
 	int ret;
 
-	if (panel->backlight.edp.vesa.luminance_control_support) {
+	ret = drm_edp_backlight_init(&intel_dp->aux,
&panel->backlight.edp.vesa.info,
+				     luminance_range, panel->vbt.backlight.pwm_freq_hz,
+				     intel_dp->edp_dpcd, &current_level, &current_mode, true);
+	if (ret < 0)
+		return ret;
+
+	drm_dbg_kms(display->drm,
+		    "[CONNECTOR:%d:%s] AUX VESA backlight enable is controlled through
%s\n",
+		    connector->base.base.id, connector->base.name,
+		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_enable));
+	drm_dbg_kms(display->drm,
+		    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled through
%s\n",
+		    connector->base.base.id, connector->base.name,
+		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
+
+	if (!panel->backlight.edp.vesa.info.luminance_set ||
+	    !panel->backlight.edp.vesa.info.aux_set ||
+	    !panel->backlight.edp.vesa.info.aux_enable) {
+		ret = panel->backlight.pwm_funcs->setup(connector, pipe);
+		if (ret < 0) {
+			drm_err(display->drm,
+				"[CONNECTOR:%d:%s] Failed to setup PWM backlight controls for eDP
backlight: %d\n",
+				connector->base.base.id, connector->base.name, ret);
+			return ret;
+		}
+	}
+
+	if (panel->backlight.edp.vesa.info.luminance_set) {
 		if (luminance_range->max_luminance) {
-			panel->backlight.max = luminance_range->max_luminance;
+			panel->backlight.max = panel->backlight.edp.vesa.info.max;
 			panel->backlight.min = luminance_range->min_luminance;
 		} else {
 			panel->backlight.max = 512;
@@ -596,57 +623,28 @@ static int intel_dp_aux_vesa_setup_backlight(struct
intel_connector *connector,
 		panel->backlight.level = intel_dp_aux_vesa_get_backlight(connector, 0);
 		panel->backlight.enabled = panel->backlight.level != 0;
 		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is controlled
through DPCD\n",
-			    connector->base.base.id, connector->base.name);
-	} else {
-		ret = drm_edp_backlight_init(&intel_dp->aux,
&panel->backlight.edp.vesa.info,
-					     luminance_range, panel->vbt.backlight.pwm_freq_hz,
-					     intel_dp->edp_dpcd, &current_level, &current_mode,
-					     false);
-		if (ret < 0)
-			return ret;
-
-		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA backlight enable is controlled through
%s\n",
-			    connector->base.base.id, connector->base.name,
-			    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_enable));
-		drm_dbg_kms(display->drm,
-			    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled through
%s\n",
-			    connector->base.base.id, connector->base.name,
-			    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
-
-		if (!panel->backlight.edp.vesa.info.aux_set ||
-		    !panel->backlight.edp.vesa.info.aux_enable) {
-			ret = panel->backlight.pwm_funcs->setup(connector, pipe);
-			if (ret < 0) {
-				drm_err(display->drm,
-					"[CONNECTOR:%d:%s] Failed to setup PWM backlight controls for eDP
backlight: %d\n",
-					connector->base.base.id, connector->base.name, ret);
-				return ret;
-			}
+		    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is controlled
through DPCD\n",
+		    connector->base.base.id, connector->base.name);
+	} else if (panel->backlight.edp.vesa.info.aux_set) {
+		panel->backlight.max = panel->backlight.edp.vesa.info.max;
+		panel->backlight.min = 0;
+		if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
+			panel->backlight.level = current_level;
+			panel->backlight.enabled = panel->backlight.level != 0;
+		} else {
+			panel->backlight.level = panel->backlight.max;
+			panel->backlight.enabled = false;
 		}
-
-		if (panel->backlight.edp.vesa.info.aux_set) {
-			panel->backlight.max = panel->backlight.edp.vesa.info.max;
-			panel->backlight.min = 0;
-			if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
-				panel->backlight.level = current_level;
-				panel->backlight.enabled = panel->backlight.level != 0;
-			} else {
-				panel->backlight.level = panel->backlight.max;
-				panel->backlight.enabled = false;
-			}
+	} else {
+		panel->backlight.max = panel->backlight.pwm_level_max;
+		panel->backlight.min = panel->backlight.pwm_level_min;
+		if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
+			panel->backlight.level +			
panel->backlight.pwm_funcs->get(connector, pipe);
+			panel->backlight.enabled = panel->backlight.pwm_enabled;
 		} else {
-			panel->backlight.max = panel->backlight.pwm_level_max;
-			panel->backlight.min = panel->backlight.pwm_level_min;
-			if (current_mode == DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
-				panel->backlight.level -				
panel->backlight.pwm_funcs->get(connector, pipe);
-				panel->backlight.enabled = panel->backlight.pwm_enabled;
-			} else {
-				panel->backlight.level = panel->backlight.max;
-				panel->backlight.enabled = false;
-			}
+			panel->backlight.level = panel->backlight.max;
+			panel->backlight.enabled = false;
 		}
 	}
 
-- 
2.34.1

Murthy, Arun R

2025-Jun-20 05:29 UTC

head link

[PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal at intel.com>
> Sent: Monday, April 14, 2025 9:47 AM
> To: nouveau at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
intel-
> xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Murthy, Arun R
> <arun.r.murthy at intel.com>; Kandpal, Suraj <suraj.kandpal at
intel.com>
> Subject: [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp
> backlight
> 
> Now that drm_edp_backlight init has been to be able to setup brightnessCan you reframe this?
> manipulation via luminance we can just use that.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>  .../drm/i915/display/intel_dp_aux_backlight.c | 100 +++++++++---------
>  1 file changed, 49 insertions(+), 51 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 be740fb72ebc..2eff9b545390 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -585,9 +585,36 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  	u8 current_mode;
>  	int ret;
> 
> -	if (panel->backlight.edp.vesa.luminance_control_support) {
> +	ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
> +				     luminance_range, panel-
> >vbt.backlight.pwm_freq_hz,
> +				     intel_dp->edp_dpcd, &current_level,
> &current_mode, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_dbg_kms(display->drm,
> +		    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> controlled through %s\n",
> +		    connector->base.base.id, connector->base.name,
> +		    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_enable));
> +	drm_dbg_kms(display->drm,
> +		    "[CONNECTOR:%d:%s] AUX VESA backlight level is controlled
> through %s\n",
> +		    connector->base.base.id, connector->base.name,
> +		    dpcd_vs_pwm_str(panel->backlight.edp.vesa.info.aux_set));
> +Is two separate debug prints required to convey that backlight is enabled and
the level is set with he same parameters?
> +	if (!panel->backlight.edp.vesa.info.luminance_set ||
> +	    !panel->backlight.edp.vesa.info.aux_set ||
> +	    !panel->backlight.edp.vesa.info.aux_enable) {
> +		ret = panel->backlight.pwm_funcs->setup(connector, pipe);If pwm is used then none of the above conditions will be true so instead can
check for the assignment of the function pointer if (pwm_funcs->setup)
In backlight struct enabled and pwm_enabled doesn't make much sense, maybe
one element to convey the mode make it easier. This can be taken out of this
series.
> +		if (ret < 0) {
> +			drm_err(display->drm,
> +				"[CONNECTOR:%d:%s] Failed to setup PWM
> backlight controls for eDP backlight: %d\n",
> +				connector->base.base.id, connector-
> >base.name, ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (panel->backlight.edp.vesa.info.luminance_set) {
>  		if (luminance_range->max_luminance) {
> -			panel->backlight.max = luminance_range-
> >max_luminance;
> +			panel->backlight.max = panel-
> >backlight.edp.vesa.info.max;
>  			panel->backlight.min = luminance_range-
> >min_luminance;
>  		} else {
>  			panel->backlight.max = 512;
> @@ -596,57 +623,28 @@ static int intel_dp_aux_vesa_setup_backlight(struct
> intel_connector *connector,
>  		panel->backlight.level >
intel_dp_aux_vesa_get_backlight(connector, 0);
>  		panel->backlight.enabled = panel->backlight.level != 0;
>  		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level
> is controlled through DPCD\n",
> -			    connector->base.base.id, connector->base.name);
> -	} else {
> -		ret = drm_edp_backlight_init(&intel_dp->aux, &panel-
> >backlight.edp.vesa.info,
> -					     luminance_range, panel-
> >vbt.backlight.pwm_freq_hz,
> -					     intel_dp->edp_dpcd,
> &current_level, &current_mode,
> -					     false);
> -		if (ret < 0)
> -			return ret;
> -
> -		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA backlight enable is
> controlled through %s\n",
> -			    connector->base.base.id, connector->base.name,
> -			    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_enable));
> -		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] AUX VESA backlight level is
> controlled through %s\n",
> -			    connector->base.base.id, connector->base.name,
> -			    dpcd_vs_pwm_str(panel-
> >backlight.edp.vesa.info.aux_set));
> -
> -		if (!panel->backlight.edp.vesa.info.aux_set ||
> -		    !panel->backlight.edp.vesa.info.aux_enable) {
> -			ret = panel->backlight.pwm_funcs->setup(connector,
> pipe);
> -			if (ret < 0) {
> -				drm_err(display->drm,
> -					"[CONNECTOR:%d:%s] Failed to setup
> PWM backlight controls for eDP backlight: %d\n",
> -					connector->base.base.id, connector-
> >base.name, ret);
> -				return ret;
> -			}
> +		    "[CONNECTOR:%d:%s] AUX VESA Nits backlight level is
> controlled through DPCD\n",
> +		    connector->base.base.id, connector->base.name);
> +	} else if (panel->backlight.edp.vesa.info.aux_set) {
> +		panel->backlight.max = panel->backlight.edp.vesa.info.max;
> +		panel->backlight.min = 0;
> +		if (current_mode => DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> +			panel->backlight.level = current_level;
> +			panel->backlight.enabled = panel->backlight.level != 0;
> +		} else {
> +			panel->backlight.level = panel->backlight.max;
> +			panel->backlight.enabled = false;
>  		}
> -
> -		if (panel->backlight.edp.vesa.info.aux_set) {
> -			panel->backlight.max = panel-
> >backlight.edp.vesa.info.max;
> -			panel->backlight.min = 0;
> -			if (current_mode => DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD) {
> -				panel->backlight.level = current_level;
> -				panel->backlight.enabled = panel-
> >backlight.level != 0;
> -			} else {
> -				panel->backlight.level = panel->backlight.max;
> -				panel->backlight.enabled = false;
> -			}
> +	} else {
> +		panel->backlight.max = panel->backlight.pwm_level_max;
> +		panel->backlight.min = panel->backlight.pwm_level_min;
> +		if (current_mode => DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> +			panel->backlight.level > +			
panel->backlight.pwm_funcs->get(connector,
> pipe);
> +			panel->backlight.enabled = panel-
> >backlight.pwm_enabled;
>  		} else {
> -			panel->backlight.max = panel-
> >backlight.pwm_level_max;
> -			panel->backlight.min = panel-
> >backlight.pwm_level_min;
> -			if (current_mode => DP_EDP_BACKLIGHT_CONTROL_MODE_PWM) {
> -				panel->backlight.level > -					panel->backlight.pwm_funcs-
> >get(connector, pipe);
> -				panel->backlight.enabled = panel-
> >backlight.pwm_enabled;
> -			} else {
> -				panel->backlight.level = panel->backlight.max;
> -				panel->backlight.enabled = false;
> -			}
> +			panel->backlight.level = panel->backlight.max;
> +			panel->backlight.enabled = false;Change is simple, adding a new condition for luminance and moving
edp_backlight_init out of the if condition so that it will be initialized for
all the modes and then initializing backlight level and max based on the mode.
But the change looks complex.

Thanks and Regards,
Arun R Murthy
------------------->  		}
>  	}
> 
> --
> 2.34.1

Nouveau - Jun 2025 - [PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

[PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight

[PATCH 11/13] drm/i915/backlight: Use drm helper to initialize edp backlight