[Intel-gfx] [PATCH v4 1/5] drm/i915/gen11: Start distinguishing 'phy' from 'port'

Souza, Jose jose.souza at intel.com
Mon Jul 8 23:59:21 UTC 2019


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 at linux.intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  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,


More information about the Intel-gfx mailing list