diff mbox series

[3/5] drm/i915/cnl+: Verify combo PHY HW state during PHY uninit

Message ID 20181102180706.16582-4-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/icl: Fix combo PHY HW context loss | expand

Commit Message

Imre Deak Nov. 2, 2018, 6:07 p.m. UTC 
Verify on CNL, ICL that the combo PHY HW state stayed intact after PHY
initialization.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103 ++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)

Comments

Souza, Jose Nov. 2, 2018, 9:25 p.m. UTC | #1 
On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Verify on CNL, ICL that the combo PHY HW state stayed intact after
> PHY
> initialization.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> ++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 13184ae5a217..1522e2a25390 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -52,8 +52,8 @@ static const struct cnl_procmon {
>   * registers, that's why we call the ICL macros even though the
> function has CNL
>   * on its name.
>   */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> -				       enum port port)
> +static const struct cnl_procmon *
> +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> port port)
>  {
>  	const struct cnl_procmon *procmon;
>  	u32 val;
> @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  		break;
>  	}
>  
> +	return procmon;
> +}
> +
> +static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
>  	val = I915_READ(ICL_PORT_COMP_DW1(port));
>  	val &= ~((0xff << 16) | 0xff);
>  	val |= procmon->dw1;
> @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
>  }
>  
> +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> +			  enum port port, i915_reg_t reg, u32 mask,
> +			  u32 expected_val)
> +{
> +	u32 val = I915_READ(reg);
> +
> +	if ((val & mask) != expected_val) {
> +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> mismatch: "
> +				 "current %08x mask %08x expected
> %08x\n",
> +				 port_name(port),
> +				 reg.reg, val, mask, expected_val);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +					  enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	bool ret;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
> +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> +			    (0xff << 16) | 0xff, procmon->dw1);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> +			     -1U, procmon->dw9);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> +			     -1U, procmon->dw10);
> +
> +	return ret;
> +}
> +
> +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> +{
> +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);


Minor but would be better add parenthesis in the first part:


return (!(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN)) &&
(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);

> +}
> +
> +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv)
> +{
> +	enum port port = PORT_A;
> +	bool ret;
> +
> +	if (!cnl_combo_phy_enabled(dev_priv))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
> @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> +	if (!cnl_combo_phy_verify_state(dev_priv))
> +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> +
>  	val = I915_READ(CHICKEN_MISC_2);
>  	val |= CNL_COMP_PWR_DOWN;
>  	I915_WRITE(CHICKEN_MISC_2, val);
>  }
>  
> +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);

Same in here:

return (!(I915_READ(ICL_PHY_MISC(port)) &
ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN)) &&
(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);

Other than that:
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> +}
> +
> +static bool icl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	bool ret;
> +
> +	if (!icl_combo_phy_enabled(dev_priv, port))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
> @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (!icl_combo_phy_verify_state(dev_priv, port))
> +			DRM_WARN("Port-%c combo PHY HW state changed
> unexpectedly\n",
> +				 port_name(port));
> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
Souza, Jose Nov. 2, 2018, 9:34 p.m. UTC | #2 
On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Verify on CNL, ICL that the combo PHY HW state stayed intact after
> PHY
> initialization.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> ++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 13184ae5a217..1522e2a25390 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -52,8 +52,8 @@ static const struct cnl_procmon {
>   * registers, that's why we call the ICL macros even though the
> function has CNL
>   * on its name.
>   */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> -				       enum port port)
> +static const struct cnl_procmon *
> +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> port port)
>  {
>  	const struct cnl_procmon *procmon;
>  	u32 val;
> @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  		break;
>  	}
>  
> +	return procmon;
> +}
> +
> +static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
>  	val = I915_READ(ICL_PORT_COMP_DW1(port));
>  	val &= ~((0xff << 16) | 0xff);
>  	val |= procmon->dw1;
> @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
>  }
>  
> +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> +			  enum port port, i915_reg_t reg, u32 mask,
> +			  u32 expected_val)
> +{
> +	u32 val = I915_READ(reg);
> +
> +	if ((val & mask) != expected_val) {
> +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> mismatch: "
> +				 "current %08x mask %08x expected
> %08x\n",
> +				 port_name(port),
> +				 reg.reg, val, mask, expected_val);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +					  enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	bool ret;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
> +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> +			    (0xff << 16) | 0xff, procmon->dw1);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> +			     -1U, procmon->dw9);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> +			     -1U, procmon->dw10);
> +
> +	return ret;
> +}
> +
> +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> +{
> +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
> +}
> +
> +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv)
> +{
> +	enum port port = PORT_A;
> +	bool ret;
> +
> +	if (!cnl_combo_phy_enabled(dev_priv))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
> @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> +	if (!cnl_combo_phy_verify_state(dev_priv))
> +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> +
>  	val = I915_READ(CHICKEN_MISC_2);
>  	val |= CNL_COMP_PWR_DOWN;
>  	I915_WRITE(CHICKEN_MISC_2, val);
>  }
>  
> +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);
> +}
> +
> +static bool icl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	bool ret;
> +
> +	if (!icl_combo_phy_enabled(dev_priv, port))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
> @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (!icl_combo_phy_verify_state(dev_priv, port))
> +			DRM_WARN("Port-%c combo PHY HW state changed
> unexpectedly\n",
> +				 port_name(port));

Minor: In most of the places the port indentification is printed like
this: "port %c"

> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
Imre Deak Nov. 2, 2018, 10 p.m. UTC | #3 
On Fri, Nov 02, 2018 at 11:25:58PM +0200, Souza, Jose wrote:
> On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> > Verify on CNL, ICL that the combo PHY HW state stayed intact after
> > PHY
> > initialization.
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> > ++++++++++++++++++++++++++++++++-
> >  1 file changed, 101 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> > b/drivers/gpu/drm/i915/intel_combo_phy.c
> > index 13184ae5a217..1522e2a25390 100644
> > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -52,8 +52,8 @@ static const struct cnl_procmon {
> >   * registers, that's why we call the ICL macros even though the
> > function has CNL
> >   * on its name.
> >   */
> > -static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > -				       enum port port)
> > +static const struct cnl_procmon *
> > +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> > port port)
> >  {
> >  	const struct cnl_procmon *procmon;
> >  	u32 val;
> > @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> > drm_i915_private *dev_priv,
> >  		break;
> >  	}
> >  
> > +	return procmon;
> > +}
> > +
> > +static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > +				       enum port port)
> > +{
> > +	const struct cnl_procmon *procmon;
> > +	u32 val;
> > +
> > +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> > +
> >  	val = I915_READ(ICL_PORT_COMP_DW1(port));
> >  	val &= ~((0xff << 16) | 0xff);
> >  	val |= procmon->dw1;
> > @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> > drm_i915_private *dev_priv,
> >  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> >  }
> >  
> > +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> > +			  enum port port, i915_reg_t reg, u32 mask,
> > +			  u32 expected_val)
> > +{
> > +	u32 val = I915_READ(reg);
> > +
> > +	if ((val & mask) != expected_val) {
> > +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> > mismatch: "
> > +				 "current %08x mask %08x expected
> > %08x\n",
> > +				 port_name(port),
> > +				 reg.reg, val, mask, expected_val);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > +					  enum port port)
> > +{
> > +	const struct cnl_procmon *procmon;
> > +	bool ret;
> > +
> > +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> > +
> > +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> > +			    (0xff << 16) | 0xff, procmon->dw1);
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> > +			     -1U, procmon->dw9);
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> > +			     -1U, procmon->dw10);
> > +
> > +	return ret;
> > +}
> > +
> > +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> > +{
> > +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> > +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
> 
> 
> Minor but would be better add parenthesis in the first part:
> 
> return (!(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN)) &&
> (I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);

That's overdoing it imo. Consider that you wouldn't do the same for
!a && b

> 
> > +}
> > +
> > +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	enum port port = PORT_A;
> > +	bool ret;
> > +
> > +	if (!cnl_combo_phy_enabled(dev_priv))
> > +		return false;
> > +
> > +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> > +
> > +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> > +			     CL_POWER_DOWN_ENABLE,
> > CL_POWER_DOWN_ENABLE);
> > +
> > +	return ret;
> > +}
> > +
> >  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val;
> > @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	u32 val;
> >  
> > +	if (!cnl_combo_phy_verify_state(dev_priv))
> > +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> > +
> >  	val = I915_READ(CHICKEN_MISC_2);
> >  	val |= CNL_COMP_PWR_DOWN;
> >  	I915_WRITE(CHICKEN_MISC_2, val);
> >  }
> >  
> > +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);
> 
> Same in here:
> 
> return (!(I915_READ(ICL_PHY_MISC(port)) &
> ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN)) &&
> (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
> 
> Other than that:
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > +}
> > +
> > +static bool icl_combo_phy_verify_state(struct drm_i915_private
> > *dev_priv,
> > +				       enum port port)
> > +{
> > +	bool ret;
> > +
> > +	if (!icl_combo_phy_enabled(dev_priv, port))
> > +		return false;
> > +
> > +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> > +
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> > +			     CL_POWER_DOWN_ENABLE,
> > CL_POWER_DOWN_ENABLE);
> > +
> > +	return ret;
> > +}
> > +
> >  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> >  {
> >  	enum port port;
> > @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> > drm_i915_private *dev_priv)
> >  	for (port = PORT_A; port <= PORT_B; port++) {
> >  		u32 val;
> >  
> > +		if (!icl_combo_phy_verify_state(dev_priv, port))
> > +			DRM_WARN("Port-%c combo PHY HW state changed
> > unexpectedly\n",
> > +				 port_name(port));
> > +
> >  		val = I915_READ(ICL_PHY_MISC(port));
> >  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> >  		I915_WRITE(ICL_PHY_MISC(port), val);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index 13184ae5a217..1522e2a25390 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -52,8 +52,8 @@  static const struct cnl_procmon {
  * registers, that's why we call the ICL macros even though the function has CNL
  * on its name.
  */
-static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
-				       enum port port)
+static const struct cnl_procmon *
+cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum port port)
 {
 	const struct cnl_procmon *procmon;
 	u32 val;
@@ -80,6 +80,17 @@  static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
 		break;
 	}
 
+	return procmon;
+}
+
+static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
+				       enum port port)
+{
+	const struct cnl_procmon *procmon;
+	u32 val;
+
+	procmon = cnl_get_procmon_ref_values(dev_priv, port);
+
 	val = I915_READ(ICL_PORT_COMP_DW1(port));
 	val &= ~((0xff << 16) | 0xff);
 	val |= procmon->dw1;
@@ -89,6 +100,63 @@  static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
 	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
 }
 
+static bool check_phy_reg(struct drm_i915_private *dev_priv,
+			  enum port port, i915_reg_t reg, u32 mask,
+			  u32 expected_val)
+{
+	u32 val = I915_READ(reg);
+
+	if ((val & mask) != expected_val) {
+		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state mismatch: "
+				 "current %08x mask %08x expected %08x\n",
+				 port_name(port),
+				 reg.reg, val, mask, expected_val);
+		return false;
+	}
+
+	return true;
+}
+
+static bool cnl_verify_procmon_ref_values(struct drm_i915_private *dev_priv,
+					  enum port port)
+{
+	const struct cnl_procmon *procmon;
+	bool ret;
+
+	procmon = cnl_get_procmon_ref_values(dev_priv, port);
+
+	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
+			    (0xff << 16) | 0xff, procmon->dw1);
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
+			     -1U, procmon->dw9);
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
+			     -1U, procmon->dw10);
+
+	return ret;
+}
+
+static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
+{
+	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
+		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
+}
+
+static bool cnl_combo_phy_verify_state(struct drm_i915_private *dev_priv)
+{
+	enum port port = PORT_A;
+	bool ret;
+
+	if (!cnl_combo_phy_enabled(dev_priv))
+		return false;
+
+	ret = cnl_verify_procmon_ref_values(dev_priv, port);
+
+	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
+			     CL_POWER_DOWN_ENABLE, CL_POWER_DOWN_ENABLE);
+
+	return ret;
+}
+
 void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	u32 val;
@@ -113,11 +181,38 @@  void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 {
 	u32 val;
 
+	if (!cnl_combo_phy_verify_state(dev_priv))
+		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
+
 	val = I915_READ(CHICKEN_MISC_2);
 	val |= CNL_COMP_PWR_DOWN;
 	I915_WRITE(CHICKEN_MISC_2, val);
 }
 
+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);
+}
+
+static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
+				       enum port port)
+{
+	bool ret;
+
+	if (!icl_combo_phy_enabled(dev_priv, port))
+		return false;
+
+	ret = cnl_verify_procmon_ref_values(dev_priv, port);
+
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
+			     CL_POWER_DOWN_ENABLE, CL_POWER_DOWN_ENABLE);
+
+	return ret;
+}
+
 void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	enum port port;
@@ -148,6 +243,10 @@  void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 	for (port = PORT_A; port <= PORT_B; port++) {
 		u32 val;
 
+		if (!icl_combo_phy_verify_state(dev_priv, port))
+			DRM_WARN("Port-%c combo PHY HW state changed unexpectedly\n",
+				 port_name(port));
+
 		val = I915_READ(ICL_PHY_MISC(port));
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);