thr3ads.net - Nouveau - [PATCH v3 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:

Kandpal, Suraj

2025-Jun-24 04:52 UTC

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

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy at intel.com>
> Sent: Tuesday, June 24, 2025 10:18 AM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>; intel-xe at
lists.freedesktop.org;
> intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> nouveau at lists.freedesktop.org
> Subject: RE: [PATCH v3 11/13] drm/i915/backlight: Use drm helper to
initialize
> edp backlight
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal at intel.com>
> > Sent: Friday, June 20, 2025 12:05 PM
> > To: intel-xe at lists.freedesktop.org; intel-gfx at
lists.freedesktop.org;
> > dri- devel at lists.freedesktop.org; nouveau at lists.freedesktop.org
> > Cc: Murthy, Arun R <arun.r.murthy at intel.com>; Kandpal, Suraj
> > <suraj.kandpal at intel.com>
> > Subject: [PATCH v3 11/13] drm/i915/backlight: Use drm helper to
> > initialize edp backlight
> >
> > Now that drm_edp_backlight init has been modified to take into account
> > the setup of lumininace based brightness manipulation we can just use
> that.
> >
> > --v2
> > -Fix commit message [Arun]
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  .../drm/i915/display/intel_dp_aux_backlight.c | 98
> > +++++++++----------
> >  1 file changed, 48 insertions(+), 50 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 800d07c7f041..117c762fa2fe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -584,9 +584,37 @@ 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->max_luminance,
> > +				     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));
> > +
> Can these both debug prints be combined?
> 
One talks about backlight enable and other about backlight level do you still
Want to combine them ?

Regards,
Suraj Kandpal
> With the above change
> Reviewed-by: Arun R Murthy <arun.r.murthy at intel.com>
> 
> Thanks and Regards,
> Arun R Murthy
> -------------------
> > +	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;
> > +		}
> > +	}
> > +
> > +	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;
> > @@ -597,56 +625,26 @@ static int
> > intel_dp_aux_vesa_setup_backlight(struct
> > intel_connector *connector,
> >  		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-
> >max_luminance,
> > -					     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;
> > -			}
> > +	} 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-24 05:01 UTC

head link

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

> > > -----Original Message-----
> > > From: Kandpal, Suraj <suraj.kandpal at intel.com>
> > > Sent: Friday, June 20, 2025 12:05 PM
> > > To: intel-xe at lists.freedesktop.org; intel-gfx at
lists.freedesktop.org;
> > > dri- devel at lists.freedesktop.org; nouveau at
lists.freedesktop.org
> > > Cc: Murthy, Arun R <arun.r.murthy at intel.com>; Kandpal,
Suraj
> > > <suraj.kandpal at intel.com>
> > > Subject: [PATCH v3 11/13] drm/i915/backlight: Use drm helper to
> > > initialize edp backlight
> > >
> > > Now that drm_edp_backlight init has been modified to take into
> > > account the setup of lumininace based brightness manipulation we
can
> > > just use
> > that.
> > >
> > > --v2
> > > -Fix commit message [Arun]
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > > ---
> > >  .../drm/i915/display/intel_dp_aux_backlight.c | 98
> > > +++++++++----------
> > >  1 file changed, 48 insertions(+), 50 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 800d07c7f041..117c762fa2fe 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > > @@ -584,9 +584,37 @@ 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->max_luminance,
> > > +				     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));
> > > +
> > Can these both debug prints be combined?
> >
> 
> One talks about backlight enable and other about backlight level do you
still
> Want to combine them ?
> Yes, can replace with "enable and level controlled by"

Thanks and Regards,
Arun R Murthy
--------------------

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

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

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