Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/dp: Add TPS4 PHY test pattern support

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 30 Nov 2023, Khaled Almahallawy <khaled.almahallawy@intel.com> wrote:
> Adding support for TPS4 (CP2520 Pattern 3) PHY pattern source tests.

A CTS connected to an older platform leads to invalid values being
written to the registers in question. In some cases probably also
invalid registers on the platform.

I understand that it's not likely someone's going to actually do that,
but from the kernel POV this is user input that can't be trusted.

> v2: rebase
>
> Bspec: 50482, 50484
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++
>  drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a1e63ab5761b..8908221edfa9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4679,6 +4679,7 @@ static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp,
>  	struct drm_dp_phy_test_params *data =
>  			&intel_dp->compliance.test_data.phytest;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	enum pipe pipe = crtc->pipe;
>  	u32 pattern_val;
>  
> @@ -4686,6 +4687,9 @@ static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp,
>  	case DP_LINK_QUAL_PATTERN_DISABLE:
>  		drm_dbg_kms(&dev_priv->drm, "Disable Phy Test Pattern\n");
>  		intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
> +		intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> +			     DP_TP_CTL_TRAIN_PAT4_SEL_MASK | DP_TP_CTL_LINK_TRAIN_MASK,
> +			     DP_TP_CTL_LINK_TRAIN_NORMAL);
>  		break;
>  	case DP_LINK_QUAL_PATTERN_D10_2:
>  		drm_dbg_kms(&dev_priv->drm, "Set D10.2 Phy Test Pattern\n");
> @@ -4733,6 +4737,13 @@ static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp,
>  			       DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
>  			       pattern_val);
>  		break;
> +	case DP_LINK_QUAL_PATTERN_CP2520_PAT_3:
> +		drm_dbg_kms(&dev_priv->drm, "Set TPS4 compliance Phy Test Pattern\n");
> +		intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
> +		intel_de_rmw(dev_priv, dp_tp_ctl_reg(encoder, crtc_state),
> +			DP_TP_CTL_TRAIN_PAT4_SEL_MASK | DP_TP_CTL_LINK_TRAIN_MASK,
> +			DP_TP_CTL_TRAIN_PAT4_SEL_TP4a | DP_TP_CTL_LINK_TRAIN_PAT4);

Please fix indentation, see checkpatch results.

> +		break;
>  	default:
>  		WARN(1, "Invalid Phy Test Pattern\n");
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 27dc903f0553..7feb1e1478ee 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5652,6 +5652,10 @@ enum skl_power_gate {
>  #define  DP_TP_CTL_MODE_SST			(0 << 27)
>  #define  DP_TP_CTL_MODE_MST			(1 << 27)
>  #define  DP_TP_CTL_FORCE_ACT			(1 << 25)
> +#define  DP_TP_CTL_TRAIN_PAT4_SEL_MASK		(3 << 19)
> +#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4a		(0 << 19)
> +#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4b		(1 << 19)
> +#define  DP_TP_CTL_TRAIN_PAT4_SEL_TP4c		(2 << 19)

Please don't use lowercase in the macro names.

>  #define  DP_TP_CTL_ENHANCED_FRAME_ENABLE	(1 << 18)
>  #define  DP_TP_CTL_FDI_AUTOTRAIN		(1 << 15)
>  #define  DP_TP_CTL_LINK_TRAIN_MASK		(7 << 8)

-- 
Jani Nikula, Intel