| Message ID | 20191212235121.668922-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Dec 12, 2019 at 03:56:38PM -0800, Lucas De Marchi wrote: > On Thu, Dec 12, 2019 at 03:51:21PM -0800, Matt Roper wrote: > > Now that the combo PHY aux power well handlers are used exclusively on > > Icelake, we can drop a bunch of the extra tests. > > > > v2: Don't try to use intel_uncore_rmw for register updates yet; there's > > pending display uncore patches that need to land first. (Lucas) > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > .../drm/i915/display/intel_display_power.c | 25 ++++++++----------- > > 1 file changed, 10 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 52f2332e0ab8..a0669dc15540 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -418,7 +418,9 @@ icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > > int pw_idx = power_well->desc->hsw.idx; > > enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx); > > u32 val; > > - int wa_idx_max; > > + > > + WARN_ON(!IS_ICELAKE(dev_priv)); > > + WARN_ON(intel_phy_is_combo(dev_priv, phy)); > > did you mean !intel_phy_is_combo()? Woops, yes! I'll send a followup shortly. > > I'm not sure I actually like the warns, we could just live without them. I feel like power wells are an area where we tend to do a lot of copy/paste/modify when defining a new platform because the tables are so large and a lot of the entries are similar or stay the same. I want to make sure we don't cargo cult the ICL-specific function onto future platforms by accident. It's easy to miss those mistakes in code review because it's hooking up a combo phy-specific table which *sounds* like the right thing to do on the surface if you don't realize the relevant functions have ICL-specific handling that we don't want to propagate forward. Granted, people are more likely to copy TGL than ICL at this point, but I figured the extra assertions wouldn't hurt. > > We should also remove _TGL_AUX_ANAOVRD1_C either in this commit or in > the previous one. Yeah, good point. I'll do that too. Matt > > Lucas De Marchi > > > > > val = I915_READ(regs->driver); > > I915_WRITE(regs->driver, val | HSW_PWR_WELL_CTL_REQ(pw_idx)); > > @@ -430,19 +432,11 @@ icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > > > > hsw_wait_for_power_well_enable(dev_priv, power_well); > > > > - /* Display WA #1178: icl, tgl */ > > - if (IS_TIGERLAKE(dev_priv)) > > - wa_idx_max = ICL_PW_CTL_IDX_AUX_C; > > - else > > - wa_idx_max = ICL_PW_CTL_IDX_AUX_B; > > - > > - if (!IS_ELKHARTLAKE(dev_priv) && > > - pw_idx >= ICL_PW_CTL_IDX_AUX_A && pw_idx <= wa_idx_max && > > - !intel_bios_is_port_edp(dev_priv, (enum port)phy)) { > > + if (pw_idx >= ICL_PW_CTL_IDX_AUX_A && pw_idx <= ICL_PW_CTL_IDX_AUX_B && > > + !intel_bios_is_port_edp(dev_priv, (enum port)phy)) > > val = I915_READ(ICL_AUX_ANAOVRD1(pw_idx)); > > val |= ICL_AUX_ANAOVRD1_ENABLE | ICL_AUX_ANAOVRD1_LDO_BYPASS; > > I915_WRITE(ICL_AUX_ANAOVRD1(pw_idx), val); > > - } > > } > > > > static void > > @@ -454,10 +448,11 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, > > enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx); > > u32 val; > > > > - if (INTEL_GEN(dev_priv) < 12) { > > - val = I915_READ(ICL_PORT_CL_DW12(phy)); > > - I915_WRITE(ICL_PORT_CL_DW12(phy), val & ~ICL_LANE_ENABLE_AUX); > > - } > > + WARN_ON(!IS_ICELAKE(dev_priv)); > > + WARN_ON(intel_phy_is_combo(dev_priv, phy)); > > + > > + val = I915_READ(ICL_PORT_CL_DW12(phy)); > > + I915_WRITE(ICL_PORT_CL_DW12(phy), val & ~ICL_LANE_ENABLE_AUX); > > > > val = I915_READ(regs->driver); > > I915_WRITE(regs->driver, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx)); > > -- > > 2.23.0 > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 52f2332e0ab8..a0669dc15540 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -418,7 +418,9 @@ icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, int pw_idx = power_well->desc->hsw.idx; enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx); u32 val; - int wa_idx_max; + + WARN_ON(!IS_ICELAKE(dev_priv)); + WARN_ON(intel_phy_is_combo(dev_priv, phy)); val = I915_READ(regs->driver); I915_WRITE(regs->driver, val | HSW_PWR_WELL_CTL_REQ(pw_idx)); @@ -430,19 +432,11 @@ icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, hsw_wait_for_power_well_enable(dev_priv, power_well); - /* Display WA #1178: icl, tgl */ - if (IS_TIGERLAKE(dev_priv)) - wa_idx_max = ICL_PW_CTL_IDX_AUX_C; - else - wa_idx_max = ICL_PW_CTL_IDX_AUX_B; - - if (!IS_ELKHARTLAKE(dev_priv) && - pw_idx >= ICL_PW_CTL_IDX_AUX_A && pw_idx <= wa_idx_max && - !intel_bios_is_port_edp(dev_priv, (enum port)phy)) { + if (pw_idx >= ICL_PW_CTL_IDX_AUX_A && pw_idx <= ICL_PW_CTL_IDX_AUX_B && + !intel_bios_is_port_edp(dev_priv, (enum port)phy)) val = I915_READ(ICL_AUX_ANAOVRD1(pw_idx)); val |= ICL_AUX_ANAOVRD1_ENABLE | ICL_AUX_ANAOVRD1_LDO_BYPASS; I915_WRITE(ICL_AUX_ANAOVRD1(pw_idx), val); - } } static void @@ -454,10 +448,11 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx); u32 val; - if (INTEL_GEN(dev_priv) < 12) { - val = I915_READ(ICL_PORT_CL_DW12(phy)); - I915_WRITE(ICL_PORT_CL_DW12(phy), val & ~ICL_LANE_ENABLE_AUX); - } + WARN_ON(!IS_ICELAKE(dev_priv)); + WARN_ON(intel_phy_is_combo(dev_priv, phy)); + + val = I915_READ(ICL_PORT_CL_DW12(phy)); + I915_WRITE(ICL_PORT_CL_DW12(phy), val & ~ICL_LANE_ENABLE_AUX); val = I915_READ(regs->driver); I915_WRITE(regs->driver, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx));
Now that the combo PHY aux power well handlers are used exclusively on Icelake, we can drop a bunch of the extra tests. v2: Don't try to use intel_uncore_rmw for register updates yet; there's pending display uncore patches that need to land first. (Lucas) Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- .../drm/i915/display/intel_display_power.c | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-)