On Mon, 2019-07-08 at 23:59 +0000, Souza, Jose wrote:
> 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.
You did that on patch 4, so I guess you can disconsider this comments.
>
> >
> > +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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
