| Message ID | 20221125173156.31689-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fake dual eDP VBT fixes | expand |
> -----Original Message----- > From: Ville Syrjala <ville.syrjala@linux.intel.com> > Sent: Friday, November 25, 2022 11:02 PM > To: intel-gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com> > Subject: [PATCH v2 4/9] drm/i915: Try to use the correct power sequencer > intiially on bxt/glk > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently on bxt/glk we just grab the power sequencer index from the VBT > data even though it may not have been parsed yet. That could lead us to > using the incorrect power sequencer during the initial panel probe. > > To avoid that let's try to read out the current state of the power sequencer > from the hardware. Unfortunately the power sequencer no longer has > anything in its registers to associate it with the port, so the best we can do is > just iterate through the power sequencers and pick the first one. This should > be sufficient for single panel cases. > > For the dual panel cases we probably need to go back to parsing the VBT > before the panel probe (and hope that panel_type=0xff is never a thing in > those cases). To that end the code always prefers the VBT panel sequencer, if > available. > > v2: Restructure a bit for upcoming icp+ dual PPS support > > Cc: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> LGTM. Reviewed-by: Animesh Manna <animesh.manna@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 22 +++-- > drivers/gpu/drm/i915/display/intel_panel.c | 1 + > drivers/gpu/drm/i915/display/intel_pps.c | 96 +++++++++++++++++-- > 3 files changed, 102 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index cc64e787e401..32e8b2fc3cc6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -330,7 +330,7 @@ struct intel_vbt_panel_data { > bool present; > bool active_low_pwm; > u8 min_brightness; /* min_brightness/255 of max */ > - u8 controller; /* brightness controller number */ > + s8 controller; /* brightness controller number */ > enum intel_backlight_type type; > } backlight; > > @@ -1570,11 +1570,19 @@ struct intel_pps { > ktime_t panel_power_off_time; > intel_wakeref_t vdd_wakeref; > > - /* > - * Pipe whose power sequencer is currently locked into > - * this port. Only relevant on VLV/CHV. > - */ > - enum pipe pps_pipe; > + union { > + /* > + * Pipe whose power sequencer is currently locked into > + * this port. Only relevant on VLV/CHV. > + */ > + enum pipe pps_pipe; > + > + /* > + * Power sequencer index. Only relevant on BXT+. > + */ > + int pps_idx; > + }; > + > /* > * Pipe currently driving the port. Used for preventing > * the use of the PPS for any pipe currentrly driving @@ -1583,7 > +1591,7 @@ struct intel_pps { > enum pipe active_pipe; > /* > * Set if the sequencer may be reset due to a power transition, > - * requiring a reinitialization. Only relevant on BXT. > + * requiring a reinitialization. Only relevant on BXT+. > */ > bool pps_reset; > struct edp_power_seq pps_delays; > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c > b/drivers/gpu/drm/i915/display/intel_panel.c > index 609fcdbd7d58..3b1004b019a8 100644 > --- a/drivers/gpu/drm/i915/display/intel_panel.c > +++ b/drivers/gpu/drm/i915/display/intel_panel.c > @@ -666,6 +666,7 @@ void intel_panel_init_alloc(struct intel_connector > *connector) > struct intel_panel *panel = &connector->panel; > > connector->panel.vbt.panel_type = -1; > + connector->panel.vbt.backlight.controller = -1; > INIT_LIST_HEAD(&panel->fixed_modes); > } > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > b/drivers/gpu/drm/i915/display/intel_pps.c > index 41ab12fcce0e..d8d2f22f3e0c 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -212,8 +212,7 @@ static int > bxt_power_sequencer_idx(struct intel_dp *intel_dp) { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > - struct intel_connector *connector = intel_dp->attached_connector; > - int backlight_controller = connector->panel.vbt.backlight.controller; > + int pps_idx = intel_dp->pps.pps_idx; > > lockdep_assert_held(&dev_priv->display.pps.mutex); > > @@ -221,7 +220,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp)); > > if (!intel_dp->pps.pps_reset) > - return backlight_controller; > + return pps_idx; > > intel_dp->pps.pps_reset = false; > > @@ -231,7 +230,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > */ > pps_init_registers(intel_dp, false); > > - return backlight_controller; > + return pps_idx; > } > > typedef bool (*pps_check)(struct drm_i915_private *dev_priv, int pps_idx); > @@ -311,6 +310,64 @@ vlv_initial_power_sequencer_setup(struct intel_dp > *intel_dp) > pipe_name(intel_dp->pps.pps_pipe)); > } > > +static int > +bxt_initial_pps_idx(struct drm_i915_private *i915, pps_check check) { > + int pps_idx, pps_num = 2; > + > + for (pps_idx = 0; pps_idx < pps_num; pps_idx++) { > + if (check(i915, pps_idx)) > + return pps_idx; > + } > + > + return -1; > +} > + > +static void > +pps_initial_setup(struct intel_dp *intel_dp) { > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > + struct intel_connector *connector = intel_dp->attached_connector; > + struct drm_i915_private *i915 = to_i915(encoder->base.dev); > + > + lockdep_assert_held(&i915->display.pps.mutex); > + > + if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { > + vlv_initial_power_sequencer_setup(intel_dp); > + return; > + } > + > + if (!IS_GEMINILAKE(i915) && !IS_BROXTON(i915)) > + return; > + > + /* first ask the VBT */ > + intel_dp->pps.pps_idx = connector->panel.vbt.backlight.controller; > + if (drm_WARN_ON(&i915->drm, intel_dp->pps.pps_idx >= 2)) > + intel_dp->pps.pps_idx = -1; > + > + /* VBT wasn't parsed yet? pick one where the panel is on */ > + if (intel_dp->pps.pps_idx < 0) > + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, > pps_has_pp_on); > + /* didn't find one? pick one where vdd is on */ > + if (intel_dp->pps.pps_idx < 0) > + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, > pps_has_vdd_on); > + /* didn't find one? pick any */ > + if (intel_dp->pps.pps_idx < 0) { > + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, pps_any); > + > + drm_dbg_kms(&i915->drm, > + "[ENCODER:%d:%s] no initial power sequencer, > assuming %d\n", > + encoder->base.base.id, encoder->base.name, > + intel_dp->pps.pps_idx); > + return; > + } > + > + drm_dbg_kms(&i915->drm, > + "[ENCODER:%d:%s] initial power sequencer: %d\n", > + encoder->base.base.id, encoder->base.name, > + intel_dp->pps.pps_idx); > +} > + > void intel_pps_reset_all(struct drm_i915_private *dev_priv) { > struct intel_encoder *encoder; > @@ -363,10 +420,10 @@ static void intel_pps_get_registers(struct intel_dp > *intel_dp, > > memset(regs, 0, sizeof(*regs)); > > - if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) > - pps_idx = bxt_power_sequencer_idx(intel_dp); > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > pps_idx = vlv_power_sequencer_pipe(intel_dp); > + else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) > + pps_idx = bxt_power_sequencer_idx(intel_dp); > > regs->pp_ctrl = PP_CONTROL(pps_idx); > regs->pp_stat = PP_STATUS(pps_idx); > @@ -1429,7 +1486,6 @@ void intel_pps_encoder_reset(struct intel_dp > *intel_dp) > > void intel_pps_init(struct intel_dp *intel_dp) { > - struct drm_i915_private *i915 = dp_to_i915(intel_dp); > intel_wakeref_t wakeref; > > intel_dp->pps.initializing = true; > @@ -1438,8 +1494,7 @@ void intel_pps_init(struct intel_dp *intel_dp) > pps_init_timestamps(intel_dp); > > with_intel_pps_lock(intel_dp, wakeref) { > - if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) > - vlv_initial_power_sequencer_setup(intel_dp); > + pps_initial_setup(intel_dp); > > pps_init_delays(intel_dp); > pps_init_registers(intel_dp, false); > @@ -1447,12 +1502,33 @@ void intel_pps_init(struct intel_dp *intel_dp) > } > } > > +static void pps_init_late(struct intel_dp *intel_dp) { > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > + struct intel_connector *connector = intel_dp->attached_connector; > + > + if (!IS_GEMINILAKE(i915) && !IS_BROXTON(i915)) > + return; > + > + drm_WARN(&i915->drm, connector->panel.vbt.backlight.controller > >= 0 && > + intel_dp->pps.pps_idx != connector- > >panel.vbt.backlight.controller, > + "[ENCODER:%d:%s] power sequencer mismatch: %d (initial) > vs. %d (VBT)\n", > + encoder->base.base.id, encoder->base.name, > + intel_dp->pps.pps_idx, connector- > >panel.vbt.backlight.controller); > + > + if (connector->panel.vbt.backlight.controller >= 0) > + intel_dp->pps.pps_idx = connector- > >panel.vbt.backlight.controller; > +} > + > void intel_pps_init_late(struct intel_dp *intel_dp) { > intel_wakeref_t wakeref; > > with_intel_pps_lock(intel_dp, wakeref) { > /* Reinit delays after per-panel info has been parsed from > VBT */ > + pps_init_late(intel_dp); > + > memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp- > >pps.pps_delays)); > pps_init_delays(intel_dp); > pps_init_registers(intel_dp, false); > -- > 2.37.4
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index cc64e787e401..32e8b2fc3cc6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -330,7 +330,7 @@ struct intel_vbt_panel_data { bool present; bool active_low_pwm; u8 min_brightness; /* min_brightness/255 of max */ - u8 controller; /* brightness controller number */ + s8 controller; /* brightness controller number */ enum intel_backlight_type type; } backlight; @@ -1570,11 +1570,19 @@ struct intel_pps { ktime_t panel_power_off_time; intel_wakeref_t vdd_wakeref; - /* - * Pipe whose power sequencer is currently locked into - * this port. Only relevant on VLV/CHV. - */ - enum pipe pps_pipe; + union { + /* + * Pipe whose power sequencer is currently locked into + * this port. Only relevant on VLV/CHV. + */ + enum pipe pps_pipe; + + /* + * Power sequencer index. Only relevant on BXT+. + */ + int pps_idx; + }; + /* * Pipe currently driving the port. Used for preventing * the use of the PPS for any pipe currentrly driving @@ -1583,7 +1591,7 @@ struct intel_pps { enum pipe active_pipe; /* * Set if the sequencer may be reset due to a power transition, - * requiring a reinitialization. Only relevant on BXT. + * requiring a reinitialization. Only relevant on BXT+. */ bool pps_reset; struct edp_power_seq pps_delays; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 609fcdbd7d58..3b1004b019a8 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -666,6 +666,7 @@ void intel_panel_init_alloc(struct intel_connector *connector) struct intel_panel *panel = &connector->panel; connector->panel.vbt.panel_type = -1; + connector->panel.vbt.backlight.controller = -1; INIT_LIST_HEAD(&panel->fixed_modes); } diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index 41ab12fcce0e..d8d2f22f3e0c 100644 --- a/drivers/gpu/drm/i915/display/intel_pps.c +++ b/drivers/gpu/drm/i915/display/intel_pps.c @@ -212,8 +212,7 @@ static int bxt_power_sequencer_idx(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); - struct intel_connector *connector = intel_dp->attached_connector; - int backlight_controller = connector->panel.vbt.backlight.controller; + int pps_idx = intel_dp->pps.pps_idx; lockdep_assert_held(&dev_priv->display.pps.mutex); @@ -221,7 +220,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp)); if (!intel_dp->pps.pps_reset) - return backlight_controller; + return pps_idx; intel_dp->pps.pps_reset = false; @@ -231,7 +230,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) */ pps_init_registers(intel_dp, false); - return backlight_controller; + return pps_idx; } typedef bool (*pps_check)(struct drm_i915_private *dev_priv, int pps_idx); @@ -311,6 +310,64 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) pipe_name(intel_dp->pps.pps_pipe)); } +static int +bxt_initial_pps_idx(struct drm_i915_private *i915, pps_check check) +{ + int pps_idx, pps_num = 2; + + for (pps_idx = 0; pps_idx < pps_num; pps_idx++) { + if (check(i915, pps_idx)) + return pps_idx; + } + + return -1; +} + +static void +pps_initial_setup(struct intel_dp *intel_dp) +{ + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; + struct intel_connector *connector = intel_dp->attached_connector; + struct drm_i915_private *i915 = to_i915(encoder->base.dev); + + lockdep_assert_held(&i915->display.pps.mutex); + + if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { + vlv_initial_power_sequencer_setup(intel_dp); + return; + } + + if (!IS_GEMINILAKE(i915) && !IS_BROXTON(i915)) + return; + + /* first ask the VBT */ + intel_dp->pps.pps_idx = connector->panel.vbt.backlight.controller; + if (drm_WARN_ON(&i915->drm, intel_dp->pps.pps_idx >= 2)) + intel_dp->pps.pps_idx = -1; + + /* VBT wasn't parsed yet? pick one where the panel is on */ + if (intel_dp->pps.pps_idx < 0) + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, pps_has_pp_on); + /* didn't find one? pick one where vdd is on */ + if (intel_dp->pps.pps_idx < 0) + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, pps_has_vdd_on); + /* didn't find one? pick any */ + if (intel_dp->pps.pps_idx < 0) { + intel_dp->pps.pps_idx = bxt_initial_pps_idx(i915, pps_any); + + drm_dbg_kms(&i915->drm, + "[ENCODER:%d:%s] no initial power sequencer, assuming %d\n", + encoder->base.base.id, encoder->base.name, + intel_dp->pps.pps_idx); + return; + } + + drm_dbg_kms(&i915->drm, + "[ENCODER:%d:%s] initial power sequencer: %d\n", + encoder->base.base.id, encoder->base.name, + intel_dp->pps.pps_idx); +} + void intel_pps_reset_all(struct drm_i915_private *dev_priv) { struct intel_encoder *encoder; @@ -363,10 +420,10 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp, memset(regs, 0, sizeof(*regs)); - if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) - pps_idx = bxt_power_sequencer_idx(intel_dp); - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) pps_idx = vlv_power_sequencer_pipe(intel_dp); + else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) + pps_idx = bxt_power_sequencer_idx(intel_dp); regs->pp_ctrl = PP_CONTROL(pps_idx); regs->pp_stat = PP_STATUS(pps_idx); @@ -1429,7 +1486,6 @@ void intel_pps_encoder_reset(struct intel_dp *intel_dp) void intel_pps_init(struct intel_dp *intel_dp) { - struct drm_i915_private *i915 = dp_to_i915(intel_dp); intel_wakeref_t wakeref; intel_dp->pps.initializing = true; @@ -1438,8 +1494,7 @@ void intel_pps_init(struct intel_dp *intel_dp) pps_init_timestamps(intel_dp); with_intel_pps_lock(intel_dp, wakeref) { - if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) - vlv_initial_power_sequencer_setup(intel_dp); + pps_initial_setup(intel_dp); pps_init_delays(intel_dp); pps_init_registers(intel_dp, false); @@ -1447,12 +1502,33 @@ void intel_pps_init(struct intel_dp *intel_dp) } } +static void pps_init_late(struct intel_dp *intel_dp) +{ + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; + struct intel_connector *connector = intel_dp->attached_connector; + + if (!IS_GEMINILAKE(i915) && !IS_BROXTON(i915)) + return; + + drm_WARN(&i915->drm, connector->panel.vbt.backlight.controller >= 0 && + intel_dp->pps.pps_idx != connector->panel.vbt.backlight.controller, + "[ENCODER:%d:%s] power sequencer mismatch: %d (initial) vs. %d (VBT)\n", + encoder->base.base.id, encoder->base.name, + intel_dp->pps.pps_idx, connector->panel.vbt.backlight.controller); + + if (connector->panel.vbt.backlight.controller >= 0) + intel_dp->pps.pps_idx = connector->panel.vbt.backlight.controller; +} + void intel_pps_init_late(struct intel_dp *intel_dp) { intel_wakeref_t wakeref; with_intel_pps_lock(intel_dp, wakeref) { /* Reinit delays after per-panel info has been parsed from VBT */ + pps_init_late(intel_dp); + memset(&intel_dp->pps.pps_delays, 0, sizeof(intel_dp->pps.pps_delays)); pps_init_delays(intel_dp); pps_init_registers(intel_dp, false);