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

Matt Atwood
2021-07-26T22:13:13Z

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