Re: [Intel-gfx] [PATCH 04/10] drm/i915/bios: Enable parse of two integrated panels backlight data

[Matt Atwood <matthew.s.atwood@intel.com>]

On Wed, Jul 21, 2021 at 10:43:32PM -0700, José Roberto de Souza wrote:
> Continuing the conversion from single integrated VBT data to two, now
> handling backlight data.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 59 +++++++++++--------
>  drivers/gpu/drm/i915/display/intel_bios.h     |  1 +
>  .../drm/i915/display/intel_dp_aux_backlight.c | 11 ++--
>  .../i915/display/intel_dsi_dcs_backlight.c    |  5 +-
>  drivers/gpu/drm/i915/display/intel_panel.c    | 32 ++++++----
>  drivers/gpu/drm/i915/display/intel_pps.c      |  8 ++-
>  drivers/gpu/drm/i915/i915_drv.h               | 18 +++---
>  7 files changed, 83 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 5906e9fa8f976..6770ed8b260be 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -441,11 +441,12 @@ parse_panel_dtd(struct drm_i915_private *i915,
>  
>  static void
>  parse_lfp_backlight(struct drm_i915_private *i915,
> -		    const struct bdb_header *bdb)
> +		    const struct bdb_header *bdb,
> +		    struct ddi_vbt_port_info *info,
> +		    int panel_index)
>  {
>  	const struct bdb_lfp_backlight_data *backlight_data;
>  	const struct lfp_backlight_data_entry *entry;
> -	int panel_type = i915->vbt.panel_type;
>  	u16 level;
>  
>  	backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
> @@ -459,38 +460,38 @@ parse_lfp_backlight(struct drm_i915_private *i915,
>  		return;
>  	}
>  
> -	entry = &backlight_data->data[panel_type];
> +	entry = &backlight_data->data[panel_index];
>  
> -	i915->vbt.backlight.present = entry->type == BDB_BACKLIGHT_TYPE_PWM;
> -	if (!i915->vbt.backlight.present) {
> +	info->backlight.present = entry->type == BDB_BACKLIGHT_TYPE_PWM;
> +	if (!info->backlight.present) {
>  		drm_dbg_kms(&i915->drm,
>  			    "PWM backlight not present in VBT (type %u)\n",
>  			    entry->type);
>  		return;
>  	}
>  
> -	i915->vbt.backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI;
> +	info->backlight.type = INTEL_BACKLIGHT_DISPLAY_DDI;
>  	if (bdb->version >= 191 &&
>  	    get_blocksize(backlight_data) >= sizeof(*backlight_data)) {
>  		const struct lfp_backlight_control_method *method;
>  
> -		method = &backlight_data->backlight_control[panel_type];
> -		i915->vbt.backlight.type = method->type;
> -		i915->vbt.backlight.controller = method->controller;
> +		method = &backlight_data->backlight_control[panel_index];
> +		info->backlight.type = method->type;
> +		info->backlight.controller = method->controller;
>  	}
>  
> -	i915->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
> -	i915->vbt.backlight.active_low_pwm = entry->active_low_pwm;
> +	info->backlight.pwm_freq_hz = entry->pwm_freq_hz;
> +	info->backlight.active_low_pwm = entry->active_low_pwm;
>  
>  	if (bdb->version >= 234) {
>  		u16 min_level;
>  		bool scale;
>  
> -		level = backlight_data->brightness_level[panel_type].level;
> -		min_level = backlight_data->brightness_min_level[panel_type].level;
> +		level = backlight_data->brightness_level[panel_index].level;
> +		min_level = backlight_data->brightness_min_level[panel_index].level;
>  
>  		if (bdb->version >= 236)
> -			scale = backlight_data->brightness_precision_bits[panel_type] == 16;
> +			scale = backlight_data->brightness_precision_bits[panel_index] == 16;
>  		else
>  			scale = level > 255;
>  
> @@ -501,20 +502,20 @@ parse_lfp_backlight(struct drm_i915_private *i915,
>  			drm_warn(&i915->drm, "Brightness min level > 255\n");
>  			level = 255;
>  		}
> -		i915->vbt.backlight.min_brightness = min_level;
> +		info->backlight.min_brightness = min_level;
>  	} else {
> -		level = backlight_data->level[panel_type];
> -		i915->vbt.backlight.min_brightness = entry->min_brightness;
> +		level = backlight_data->level[panel_index];
> +		info->backlight.min_brightness = entry->min_brightness;
>  	}
>  
>  	drm_dbg_kms(&i915->drm,
>  		    "VBT backlight PWM modulation frequency %u Hz, "
>  		    "active %s, min brightness %u, level %u, controller %u\n",
> -		    i915->vbt.backlight.pwm_freq_hz,
> -		    i915->vbt.backlight.active_low_pwm ? "low" : "high",
> -		    i915->vbt.backlight.min_brightness,
> +		    info->backlight.pwm_freq_hz,
> +		    info->backlight.active_low_pwm ? "low" : "high",
> +		    info->backlight.min_brightness,
>  		    level,
> -		    i915->vbt.backlight.controller);
> +		    info->backlight.controller);
>  }
>  
>  /* Try to find sdvo panel data */
> @@ -1987,6 +1988,7 @@ static void parse_integrated_panel(struct drm_i915_private *i915,
>  	parse_power_conservation_features(i915, bdb, info, panel_index);
>  	parse_driver_features_drrs_only(i915, bdb, info);
>  	parse_panel_dtd(i915, bdb, info, panel_index);
> +	parse_lfp_backlight(i915, bdb, info, panel_index);
>  }
>  
>  static void parse_ddi_port(struct drm_i915_private *i915,
> @@ -2120,6 +2122,9 @@ static void parse_ddi_port(struct drm_i915_private *i915,
>  			    port_name(port), info->dp_max_link_rate);
>  	}
>  
> +	/* Default to having backlight */
> +	info->backlight.present = true;
> +
>  	parse_integrated_panel(i915, devdata, info);
>  
>  	info->devdata = devdata;
> @@ -2245,9 +2250,6 @@ init_vbt_defaults(struct drm_i915_private *i915)
>  {
>  	i915->vbt.crt_ddc_pin = GMBUS_PIN_VGADDC;
>  
> -	/* Default to having backlight */
> -	i915->vbt.backlight.present = true;
> -
>  	/* SDVO panel data */
>  	i915->vbt.sdvo_lvds_vbt_mode = NULL;
>  
> @@ -2481,7 +2483,6 @@ void intel_bios_init(struct drm_i915_private *i915)
>  	parse_general_features(i915, bdb);
>  	parse_general_definitions(i915, bdb);
>  	parse_panel_type(i915, bdb);
> -	parse_lfp_backlight(i915, bdb);
>  	parse_sdvo_panel_data(i915, bdb);
>  	parse_driver_features(i915, bdb);
>  	parse_edp(i915, bdb);
> @@ -3123,3 +3124,11 @@ intel_bios_lfp_lvds_info(struct intel_encoder *encoder)
>  
>  	return i915->vbt.ddi_port_info[encoder->port].lfp_lvds_vbt_mode;
>  }
> +
> +const struct vbt_backlight_info *
> +intel_bios_backlight_info(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> +	return &i915->vbt.ddi_port_info[encoder->port].backlight;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index f133c51c155cd..5b6167c97a8d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -268,5 +268,6 @@ int intel_bios_encoder_hdmi_boost_level(const struct intel_bios_encoder_data *de
>  
>  enum drrs_support_type intel_bios_drrs_type(struct intel_encoder *encoder);
>  const struct drm_display_mode *intel_bios_lfp_lvds_info(struct intel_encoder *encoder);
> +const struct vbt_backlight_info *intel_bios_backlight_info(struct intel_encoder *encoder);
>  
>  #endif /* _INTEL_BIOS_H_ */
> 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 6ac568617ef37..401a9d9533158 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -310,13 +310,14 @@ static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector,
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_panel *panel = &connector->panel;
> -	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	const struct vbt_backlight_info *backlight_info;
>  	u16 current_level;
>  	u8 current_mode;
>  	int ret;
>  
> +	backlight_info = intel_bios_backlight_info(connector->encoder);
>  	ret = drm_edp_backlight_init(&intel_dp->aux, &panel->backlight.edp.vesa.info,
> -				     i915->vbt.backlight.pwm_freq_hz, intel_dp->edp_dpcd,
> +				     backlight_info->pwm_freq_hz, intel_dp->edp_dpcd,
>  				     &current_level, &current_mode);
>  	if (ret < 0)
>  		return ret;
> @@ -383,7 +384,9 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
>  	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	bool try_intel_interface = false, try_vesa_interface = false;
> +	const struct vbt_backlight_info *backlight_info;
>  
> +	backlight_info = intel_bios_backlight_info(connector->encoder);
>  	/* Check the VBT and user's module parameters to figure out which
>  	 * interfaces to probe
>  	 */
> @@ -391,7 +394,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
>  	case INTEL_DP_AUX_BACKLIGHT_OFF:
>  		return -ENODEV;
>  	case INTEL_DP_AUX_BACKLIGHT_AUTO:
> -		switch (i915->vbt.backlight.type) {
> +		switch (backlight_info->type) {
>  		case INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE:
>  			try_vesa_interface = true;
>  			break;
> @@ -403,7 +406,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
>  		}
>  		break;
>  	case INTEL_DP_AUX_BACKLIGHT_ON:
> -		if (i915->vbt.backlight.type != INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE)
> +		if (backlight_info->type != INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE)
>  			try_intel_interface = true;
>  
>  		try_vesa_interface = true;
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> index 584c14c4cbd0e..4dbd0c6754af9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> @@ -166,11 +166,12 @@ static const struct intel_panel_bl_funcs dcs_bl_funcs = {
>  int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector)
>  {
>  	struct drm_device *dev = intel_connector->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *encoder = intel_attached_encoder(intel_connector);
>  	struct intel_panel *panel = &intel_connector->panel;
> +	const struct vbt_backlight_info *backlight_info;
>  
> -	if (dev_priv->vbt.backlight.type != INTEL_BACKLIGHT_DSI_DCS)
> +	backlight_info = intel_bios_backlight_info(encoder);
> +	if (backlight_info->type != INTEL_BACKLIGHT_DSI_DCS)
>  		return -ENODEV;
>  
>  	if (drm_WARN_ON(dev, encoder->type != INTEL_OUTPUT_DSI))
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index a88e30c966fe7..9c892476d8621 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -1596,9 +1596,14 @@ static u32 vlv_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
>  	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
>  }
>  
> -static u16 get_vbt_pwm_freq(struct drm_i915_private *dev_priv)
> +static u16 get_vbt_pwm_freq(struct intel_connector *connector)
>  {
> -	u16 pwm_freq_hz = dev_priv->vbt.backlight.pwm_freq_hz;
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	const struct vbt_backlight_info *backlight_info;
> +	u16 pwm_freq_hz;
> +
> +	backlight_info = intel_bios_backlight_info(connector->encoder);
> +	pwm_freq_hz = backlight_info->pwm_freq_hz;
>  
>  	if (pwm_freq_hz) {
>  		drm_dbg_kms(&dev_priv->drm,
> @@ -1618,7 +1623,7 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	u16 pwm_freq_hz = get_vbt_pwm_freq(dev_priv);
> +	u16 pwm_freq_hz = get_vbt_pwm_freq(connector);
>  	u32 pwm;
>  
>  	if (!panel->backlight.pwm_funcs->hz_to_pwm) {
> @@ -1643,11 +1648,14 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector)
>  static u32 get_backlight_min_vbt(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	const struct vbt_backlight_info *backlight_info;
>  	struct intel_panel *panel = &connector->panel;
>  	int min;
>  
>  	drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_level_max == 0);
>  
> +	backlight_info = intel_bios_backlight_info(connector->encoder);
> +
>  	/*
>  	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
>  	 * to problems. There are such machines out there. Either our
> @@ -1655,11 +1663,11 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>  	 * against this by letting the minimum be at most (arbitrarily chosen)
>  	 * 25% of the max.
>  	 */
> -	min = clamp_t(int, dev_priv->vbt.backlight.min_brightness, 0, 64);
> -	if (min != dev_priv->vbt.backlight.min_brightness) {
> +	min = clamp_t(int, backlight_info->min_brightness, 0, 64);
> +	if (min != backlight_info->min_brightness) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "clamping VBT min backlight %d/255 to %d/255\n",
> -			    dev_priv->vbt.backlight.min_brightness, min);
> +			    backlight_info->min_brightness, min);
>  	}
>  
>  	/* vbt value is a coefficient in range [0..255] */
> @@ -1845,10 +1853,12 @@ static int
>  bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	const struct vbt_backlight_info *backlight_info;
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pwm_ctl, val;
>  
> -	panel->backlight.controller = dev_priv->vbt.backlight.controller;
> +	backlight_info = intel_bios_backlight_info(connector->encoder);
> +	panel->backlight.controller = backlight_info->controller;
>  
>  	pwm_ctl = intel_de_read(dev_priv,
>  				BXT_BLC_PWM_CTL(panel->backlight.controller));
> @@ -1950,11 +1960,11 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector,
>  
>  		drm_dbg_kms(&dev_priv->drm, "PWM already enabled at freq %ld, VBT freq %d, level %d\n",
>  			    NSEC_PER_SEC / (unsigned long)panel->backlight.pwm_state.period,
> -			    get_vbt_pwm_freq(dev_priv), level);
> +			    get_vbt_pwm_freq(connector), level);
>  	} else {
>  		/* Set period from VBT frequency, leave other settings at 0. */
>  		panel->backlight.pwm_state.period =
> -			NSEC_PER_SEC / get_vbt_pwm_freq(dev_priv);
> +			NSEC_PER_SEC / get_vbt_pwm_freq(connector);
>  	}
>  
>  	drm_info(&dev_priv->drm, "Using %s PWM for LCD backlight control\n",
> @@ -2037,10 +2047,12 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	const struct vbt_backlight_info *backlight_info;
>  	struct intel_panel *panel = &intel_connector->panel;
>  	int ret;
>  
> -	if (!dev_priv->vbt.backlight.present) {
> +	backlight_info = intel_bios_backlight_info(intel_connector->encoder);
> +	if (!backlight_info->present) {
>  		if (dev_priv->quirks & QUIRK_BACKLIGHT_PRESENT) {
>  			drm_dbg_kms(&dev_priv->drm,
>  				    "no backlight present per VBT, but present per quirk\n");
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index a36ec4a818ff5..96894d70a92c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -207,7 +207,13 @@ static int
>  bxt_power_sequencer_idx(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	int backlight_controller = dev_priv->vbt.backlight.controller;
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *encoder = &dig_port->base;
> +	const struct vbt_backlight_info *backlight_info;
> +	int backlight_controller;
> +
> +	backlight_info = intel_bios_backlight_info(encoder);
> +	backlight_controller = backlight_info->controller;
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 62a0c1f64f870..047f0d2fc971f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -660,6 +660,15 @@ struct ddi_vbt_port_info {
>  	enum drrs_support_type drrs_type;
>  
>  	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
> +
> +	struct vbt_backlight_info {
> +		u16 pwm_freq_hz;
> +		bool present;
> +		bool active_low_pwm;
> +		u8 min_brightness;	/* min_brightness/255 of max */
> +		u8 controller;		/* brightness controller number */
> +		enum intel_backlight_type type;
> +	} backlight;
>  };
>  
>  enum psr_lines_to_wait {
> @@ -710,15 +719,6 @@ struct intel_vbt_data {
>  		int psr2_tp2_tp3_wakeup_time_us;
>  	} psr;
>  
> -	struct {
> -		u16 pwm_freq_hz;
> -		bool present;
> -		bool active_low_pwm;
> -		u8 min_brightness;	/* min_brightness/255 of max */
> -		u8 controller;		/* brightness controller number */
> -		enum intel_backlight_type type;
> -	} backlight;
> -
>  	/* MIPI DSI */
>  	struct {
>  		u16 panel_id;
> -- 
> 2.32.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx