| Message ID | 20190626000352.31926-4-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EHL port programming | expand |
On Tue, 2019-06-25 at 17:03 -0700, Matt Roper wrote: > Although EHL added a third combo PHY, no PHY_MISC register was added > for > PHY C. The bspec indicates that there's no need to program the "DE > to > IO Comp Pwr Down" setting for this PHY that we usually need to set in > PHY_MISC. > > v2: > - Add IS_ELKHARTLAKE() guards since future platforms that have a PHY > C > are likely to reinstate the PHY_MISC register. (Jose) > - Use goto's to skip PHY_MISC programming & minimize code deltas. > (Jose) > Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > Bspec: 33148 > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > .../gpu/drm/i915/display/intel_combo_phy.c | 28 > +++++++++++++++++-- > 1 file changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c > b/drivers/gpu/drm/i915/display/intel_combo_phy.c > index 075bab2500eb..d3d5244765e6 100644 > --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c > @@ -183,9 +183,13 @@ static void cnl_combo_phys_uninit(struct > drm_i915_private *dev_priv) > static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv, > enum port port) > { > - return !(I915_READ(ICL_PHY_MISC(port)) & > - ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > - (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT); > + /* The PHY C added by EHL has no PHY_MISC register */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT; > + else > + return !(I915_READ(ICL_PHY_MISC(port)) & > + ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > + (I915_READ(ICL_PORT_COMP_DW0(port)) & > COMP_INIT); > } > > static bool icl_combo_phy_verify_state(struct drm_i915_private > *dev_priv, > @@ -299,6 +303,14 @@ static void icl_combo_phys_init(struct > drm_i915_private *dev_priv) > continue; > } > > + /* > + * Although EHL adds a combo PHY C, there's no PHY_MISC > + * register for it and no need to program the > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > + */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + goto skip_phy_misc; > + > /* > * EHL's combo PHY A can be hooked up to either an > external > * display (via DDI-D) or an internal display (via DDI- > A or > @@ -313,6 +325,7 @@ static void icl_combo_phys_init(struct > drm_i915_private *dev_priv) > val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > I915_WRITE(ICL_PHY_MISC(port), val); > > +skip_phy_misc: > cnl_set_procmon_ref_values(dev_priv, port); > > if (port == PORT_A) { > @@ -343,10 +356,19 @@ static void icl_combo_phys_uninit(struct > drm_i915_private *dev_priv) > DRM_WARN("Port %c combo PHY HW state changed > unexpectedly\n", > port_name(port)); > > + /* > + * Although EHL adds a combo PHY C, there's no PHY_MISC > + * register for it and no need to program the > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > + */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + goto skip_phy_misc; > + > val = I915_READ(ICL_PHY_MISC(port)); > val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > I915_WRITE(ICL_PHY_MISC(port), val); > > +skip_phy_misc: > val = I915_READ(ICL_PORT_COMP_DW0(port)); > val &= ~COMP_INIT; > I915_WRITE(ICL_PORT_COMP_DW0(port), val);
On Tue, 2019-06-25 at 17:03 -0700, Matt Roper wrote: > Although EHL added a third combo PHY, no PHY_MISC register was added > for > PHY C. The bspec indicates that there's no need to program the "DE > to > IO Comp Pwr Down" setting for this PHY that we usually need to set in > PHY_MISC. > > v2: > - Add IS_ELKHARTLAKE() guards since future platforms that have a PHY > C > are likely to reinstate the PHY_MISC register. (Jose) > - Use goto's to skip PHY_MISC programming & minimize code deltas. > (Jose) I found more registers that also needs the conversion, here a few: dsi_program_swing_and_deemphasis() ICL_PORT_TX_DW5_LN0 icl_ddi_combo_vswing_program() ICL_PORT_TX_DW5_LN0 ICL_PORT_TX_DW4_LN icl_combo_phy_ddi_vswing_sequence() ICL_PORT_TX_DW4_LN ICL_PORT_CL_DW5 ICL_PORT_TX_DW4_LN ICL_PORT_TX_DW5_GRP Suggestion: send the other patches separated, merge those patches and then we work on this one. > > Bspec: 33148 > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > .../gpu/drm/i915/display/intel_combo_phy.c | 28 > +++++++++++++++++-- > 1 file changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c > b/drivers/gpu/drm/i915/display/intel_combo_phy.c > index 075bab2500eb..d3d5244765e6 100644 > --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c > @@ -183,9 +183,13 @@ static void cnl_combo_phys_uninit(struct > drm_i915_private *dev_priv) > static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv, > enum port port) > { > - return !(I915_READ(ICL_PHY_MISC(port)) & > - ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > - (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT); > + /* The PHY C added by EHL has no PHY_MISC register */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT; > + else > + return !(I915_READ(ICL_PHY_MISC(port)) & > + ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > + (I915_READ(ICL_PORT_COMP_DW0(port)) & > COMP_INIT); > } > > static bool icl_combo_phy_verify_state(struct drm_i915_private > *dev_priv, > @@ -299,6 +303,14 @@ static void icl_combo_phys_init(struct > drm_i915_private *dev_priv) > continue; > } > > + /* > + * Although EHL adds a combo PHY C, there's no PHY_MISC > + * register for it and no need to program the > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > + */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + goto skip_phy_misc; > + > /* > * EHL's combo PHY A can be hooked up to either an > external > * display (via DDI-D) or an internal display (via DDI- > A or > @@ -313,6 +325,7 @@ static void icl_combo_phys_init(struct > drm_i915_private *dev_priv) > val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > I915_WRITE(ICL_PHY_MISC(port), val); > > +skip_phy_misc: > cnl_set_procmon_ref_values(dev_priv, port); > > if (port == PORT_A) { > @@ -343,10 +356,19 @@ static void icl_combo_phys_uninit(struct > drm_i915_private *dev_priv) > DRM_WARN("Port %c combo PHY HW state changed > unexpectedly\n", > port_name(port)); > > + /* > + * Although EHL adds a combo PHY C, there's no PHY_MISC > + * register for it and no need to program the > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > + */ > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > + goto skip_phy_misc; > + > val = I915_READ(ICL_PHY_MISC(port)); > val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > I915_WRITE(ICL_PHY_MISC(port), val); > > +skip_phy_misc: > val = I915_READ(ICL_PORT_COMP_DW0(port)); > val &= ~COMP_INIT; > I915_WRITE(ICL_PORT_COMP_DW0(port), val);
On Fri, Jun 28, 2019 at 04:52:31PM -0700, Souza, Jose wrote: > On Tue, 2019-06-25 at 17:03 -0700, Matt Roper wrote: > > Although EHL added a third combo PHY, no PHY_MISC register was added > > for > > PHY C. The bspec indicates that there's no need to program the "DE > > to > > IO Comp Pwr Down" setting for this PHY that we usually need to set in > > PHY_MISC. > > > > v2: > > - Add IS_ELKHARTLAKE() guards since future platforms that have a PHY > > C > > are likely to reinstate the PHY_MISC register. (Jose) > > - Use goto's to skip PHY_MISC programming & minimize code deltas. > > (Jose) > > I found more registers that also needs the conversion, here a few: > > dsi_program_swing_and_deemphasis() > ICL_PORT_TX_DW5_LN0 > > icl_ddi_combo_vswing_program() > ICL_PORT_TX_DW5_LN0 > ICL_PORT_TX_DW4_LN > > icl_combo_phy_ddi_vswing_sequence() > ICL_PORT_TX_DW4_LN > ICL_PORT_CL_DW5 > ICL_PORT_TX_DW4_LN > ICL_PORT_TX_DW5_GRP > > > Suggestion: send the other patches separated, merge those patches and > then we work on this one. Sounds good. Although I think you meant to leave this comment on patch #4 (port -> phy) rather than patch #3 (skip programming a specific register), right? Since patches #1-3 all have r-b's and the series as a whole has a clean CI, I'll go ahead and apply those three, then work on respinning this one. I'll hold off applying #5 until #4 is ready; no point enabling DDI-D until its PHY programming gets routed to the proper registers/bits. Matt > > > > > > Bspec: 33148 > > Cc: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > .../gpu/drm/i915/display/intel_combo_phy.c | 28 > > +++++++++++++++++-- > > 1 file changed, 25 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c > > b/drivers/gpu/drm/i915/display/intel_combo_phy.c > > index 075bab2500eb..d3d5244765e6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c > > +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c > > @@ -183,9 +183,13 @@ static void cnl_combo_phys_uninit(struct > > drm_i915_private *dev_priv) > > static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv, > > enum port port) > > { > > - return !(I915_READ(ICL_PHY_MISC(port)) & > > - ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > > - (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT); > > + /* The PHY C added by EHL has no PHY_MISC register */ > > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > > + return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT; > > + else > > + return !(I915_READ(ICL_PHY_MISC(port)) & > > + ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && > > + (I915_READ(ICL_PORT_COMP_DW0(port)) & > > COMP_INIT); > > } > > > > static bool icl_combo_phy_verify_state(struct drm_i915_private > > *dev_priv, > > @@ -299,6 +303,14 @@ static void icl_combo_phys_init(struct > > drm_i915_private *dev_priv) > > continue; > > } > > > > + /* > > + * Although EHL adds a combo PHY C, there's no PHY_MISC > > + * register for it and no need to program the > > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > > + */ > > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > > + goto skip_phy_misc; > > + > > /* > > * EHL's combo PHY A can be hooked up to either an > > external > > * display (via DDI-D) or an internal display (via DDI- > > A or > > @@ -313,6 +325,7 @@ static void icl_combo_phys_init(struct > > drm_i915_private *dev_priv) > > val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > > I915_WRITE(ICL_PHY_MISC(port), val); > > > > +skip_phy_misc: > > cnl_set_procmon_ref_values(dev_priv, port); > > > > if (port == PORT_A) { > > @@ -343,10 +356,19 @@ static void icl_combo_phys_uninit(struct > > drm_i915_private *dev_priv) > > DRM_WARN("Port %c combo PHY HW state changed > > unexpectedly\n", > > port_name(port)); > > > > + /* > > + * Although EHL adds a combo PHY C, there's no PHY_MISC > > + * register for it and no need to program the > > + * DE_IO_COMP_PWR_DOWN setting on PHY C. > > + */ > > + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) > > + goto skip_phy_misc; > > + > > val = I915_READ(ICL_PHY_MISC(port)); > > val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; > > I915_WRITE(ICL_PHY_MISC(port), val); > > > > +skip_phy_misc: > > val = I915_READ(ICL_PORT_COMP_DW0(port)); > > val &= ~COMP_INIT; > > I915_WRITE(ICL_PORT_COMP_DW0(port), val);
diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c b/drivers/gpu/drm/i915/display/intel_combo_phy.c index 075bab2500eb..d3d5244765e6 100644 --- a/drivers/gpu/drm/i915/display/intel_combo_phy.c +++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c @@ -183,9 +183,13 @@ static void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv) static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv, enum port port) { - return !(I915_READ(ICL_PHY_MISC(port)) & - ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && - (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT); + /* The PHY C added by EHL has no PHY_MISC register */ + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) + return I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT; + else + return !(I915_READ(ICL_PHY_MISC(port)) & + ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) && + (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT); } static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv, @@ -299,6 +303,14 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv) continue; } + /* + * Although EHL adds a combo PHY C, there's no PHY_MISC + * register for it and no need to program the + * DE_IO_COMP_PWR_DOWN setting on PHY C. + */ + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) + goto skip_phy_misc; + /* * EHL's combo PHY A can be hooked up to either an external * display (via DDI-D) or an internal display (via DDI-A or @@ -313,6 +325,7 @@ static void icl_combo_phys_init(struct drm_i915_private *dev_priv) val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; I915_WRITE(ICL_PHY_MISC(port), val); +skip_phy_misc: cnl_set_procmon_ref_values(dev_priv, port); if (port == PORT_A) { @@ -343,10 +356,19 @@ static void icl_combo_phys_uninit(struct drm_i915_private *dev_priv) DRM_WARN("Port %c combo PHY HW state changed unexpectedly\n", port_name(port)); + /* + * Although EHL adds a combo PHY C, there's no PHY_MISC + * register for it and no need to program the + * DE_IO_COMP_PWR_DOWN setting on PHY C. + */ + if (IS_ELKHARTLAKE(dev_priv) && port == PORT_C) + goto skip_phy_misc; + val = I915_READ(ICL_PHY_MISC(port)); val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN; I915_WRITE(ICL_PHY_MISC(port), val); +skip_phy_misc: val = I915_READ(ICL_PORT_COMP_DW0(port)); val &= ~COMP_INIT; I915_WRITE(ICL_PORT_COMP_DW0(port), val);
Although EHL added a third combo PHY, no PHY_MISC register was added for PHY C. The bspec indicates that there's no need to program the "DE to IO Comp Pwr Down" setting for this PHY that we usually need to set in PHY_MISC. v2: - Add IS_ELKHARTLAKE() guards since future platforms that have a PHY C are likely to reinstate the PHY_MISC register. (Jose) - Use goto's to skip PHY_MISC programming & minimize code deltas. (Jose) Bspec: 33148 Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- .../gpu/drm/i915/display/intel_combo_phy.c | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-)