On Wed, 2019-07-03 at 16:37 -0700, Matt Roper wrote: > Our past DDI-based Intel platforms have had a fixed DDI<->PHY > mapping. > Because of this, both the bspec documentation and our i915 code has > used > the term "port" when talking about either DDI's or PHY's; it was > always > easy to tell what terms like "Port A" were referring to from the > context. > > Unfortunately this is starting to break down now that EHL allows PHY- > A > to be driven by either DDI-A or DDI-D. Is a setup with DDI-D driving > PHY-A considered "Port A" or "Port D?" The answer depends on which > register we're working with, and even the bspec doesn't do a great > job > of clarifying this. > > Let's try to be more explicit about whether we're talking about the > DDI > or the PHY on gen11+ by using 'port' to refer to the DDI and creating > a > new 'enum phy' namespace to refer to the PHY in use. > > This patch just adds the new PHY namespace, new phy-based versions of > intel_port_is_*(), and a helper to convert a port to a PHY. > Transitioning various areas of the code over to using the PHY > namespace > will be done in subsequent patches to make review easier. We'll > remove > the intel_port_is_*() functions at the end of the series when we > transition all callers over to using the PHY-based versions. > > v2: > - Convert a few more 'port' uses to 'phy.' (Sparse) > > v3: > - Switch DDI_CLK_SEL() back to 'port.' (Jose) > - Add a code comment clarifying why DPCLKA_CFGCR0_ICL needs to use > PHY > for its bit definitions, even though the register description is > given in terms of DDI. > - To avoid confusion, switch CNL's DPCLKA_CFGCR0 defines back to > using > port and create separate ICL+ definitions that work in terms of > PHY. > > v4: > - Rebase and resolve conflicts with Imre's TC series. > - This patch now just adds the namespace and a few convenience > functions; the important changes are now split out into separate > patches to make review easier. > > Suggested-by: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Imre Deak <imre.deak@xxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_display.c | 32 > +++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_display.h | 16 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > 3 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 919f5ac844c8..4a85abef93e7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6663,6 +6663,20 @@ bool intel_port_is_combophy(struct > drm_i915_private *dev_priv, enum port port) > return false; > } A call to intel_port_is_combophy(PORT_D) would return false on EHL, it and intel_port_is_tc() should use intel_phy functions, like: bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port) { return intel_phy_is_combo(dev_priv, intel_port_to_phy(dev_priv, port)); } Even better would be check if we can replace those with intel_phy counterparts. > > +bool intel_phy_is_combo(struct drm_i915_private *dev_priv, enum phy > phy) > +{ > + if (phy == PHY_NONE) > + return false; > + > + if (IS_ELKHARTLAKE(dev_priv)) > + return phy <= PHY_C; > + > + if (INTEL_GEN(dev_priv) >= 11) > + return phy <= PHY_B; > + > + return false; > +} > + > bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port > port) > { > if (INTEL_GEN(dev_priv) >= 11 && !IS_ELKHARTLAKE(dev_priv)) > @@ -6671,9 +6685,25 @@ bool intel_port_is_tc(struct drm_i915_private > *dev_priv, enum port port) > return false; > } > > +bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy > phy) > +{ > + if (INTEL_GEN(dev_priv) >= 11 && !IS_ELKHARTLAKE(dev_priv)) > + return phy >= PHY_C && phy <= PHY_F; > + > + return false; > +} > + > +enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port > port) > +{ > + if (IS_ELKHARTLAKE(i915) && port == PORT_D) > + return PHY_A; > + > + return (enum phy)port; > +} > + > enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv, > enum port port) > { > - if (!intel_port_is_tc(dev_priv, port)) > + if (!intel_phy_is_tc(dev_priv, intel_port_to_phy(dev_priv, > port))) > return PORT_TC_NONE; > > return port - PORT_C; > diff --git a/drivers/gpu/drm/i915/display/intel_display.h > b/drivers/gpu/drm/i915/display/intel_display.h > index d296556ed82e..d53285fb883f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.h > +++ b/drivers/gpu/drm/i915/display/intel_display.h > @@ -228,6 +228,21 @@ struct intel_link_m_n { > u32 link_n; > }; > > +enum phy { > + PHY_NONE = -1, > + > + PHY_A = 0, > + PHY_B, > + PHY_C, > + PHY_D, > + PHY_E, > + PHY_F, > + > + I915_MAX_PHYS > +}; > + > +#define phy_name(a) ((a) + 'A') > + > #define for_each_pipe(__dev_priv, __p) \ > for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; > (__p)++) > > @@ -356,5 +371,6 @@ void lpt_disable_clkout_dp(struct > drm_i915_private *dev_priv); > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, > u32 pixel_format, u64 modifier); > bool intel_plane_can_remap(const struct intel_plane_state > *plane_state); > +enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port > port); > > #endif > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 24c63ed45c6f..815c26c0b98c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1493,7 +1493,9 @@ void intel_encoder_destroy(struct drm_encoder > *encoder); > struct drm_display_mode * > intel_encoder_current_mode(struct intel_encoder *encoder); > bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum > port port); > +bool intel_phy_is_combo(struct drm_i915_private *dev_priv, enum phy > phy); > bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port > port); > +bool intel_phy_is_tc(struct drm_i915_private *dev_priv, enum phy > phy); > enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv, > enum port port); > int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void > *data, _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx