Re: [Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for JSL

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Vidya Srinivas <vidya.srinivas@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for JSL
Date: Thu, 4 Jun 2020 22:06:12 +0300	[thread overview]
Message-ID: <20200604190612.GI6112@intel.com> (raw)
In-Reply-To: <1591247028-5868-1-git-send-email-vidya.srinivas@intel.com>

On Thu, Jun 04, 2020 at 10:33:48AM +0530, Vidya Srinivas wrote:
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 40 ++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7223367171d1..44663e8ac9a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5470,22 +5470,32 @@ intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
>  	enum pipe pipe = crtc->pipe;
> -	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> +	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value, trans_ddi_port_mask;
> +	enum port port = intel_dig_port->base.port;
> +	i915_reg_t dp_tp_reg;
> +
> +	if (IS_ELKHARTLAKE(dev_priv)) {
> +		dp_tp_reg = DP_TP_CTL(port);
> +		trans_ddi_port_mask = TRANS_DDI_PORT_MASK;
> +	} else if (IS_TIGERLAKE(dev_priv)) {
> +		dp_tp_reg = TGL_DP_TP_CTL(pipe);
> +		trans_ddi_port_mask = TGL_TRANS_DDI_PORT_MASK;
> +	}
>  
>  	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
>  						 TRANS_DDI_FUNC_CTL(pipe));
>  	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
> -	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
>  
> +	dp_tp_ctl_value = intel_de_read(dev_priv, dp_tp_reg);
>  	trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> -				      TGL_TRANS_DDI_PORT_MASK);
> +					trans_ddi_port_mask);
>  	trans_conf_value &= ~PIPECONF_ENABLE;
>  	dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
>  
>  	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
>  	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
>  		       trans_ddi_func_ctl_value);
> -	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> +	intel_de_write(dev_priv, dp_tp_reg, dp_tp_ctl_value);

All this ad-hoc modeset code really should not exist. It's going to
have different bugs than the norma modeset paths, so compliance testing
this special code proves absolutely nothing about the normal modeset
code. IMO someone needs to take up the task of rewrtiting all this to
just perform normal modesets.

>  }
>  
>  static void
> @@ -5497,20 +5507,28 @@ intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
>  	enum port port = intel_dig_port->base.port;
>  	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
>  	enum pipe pipe = crtc->pipe;
> -	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> +	u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value, trans_ddi_sel_port;
> +	i915_reg_t dp_tp_reg;
> +
> +	if (IS_ELKHARTLAKE(dev_priv)) {
> +		dp_tp_reg = DP_TP_CTL(port);
> +		trans_ddi_sel_port = TRANS_DDI_SELECT_PORT(port);
> +	} else if (IS_TIGERLAKE(dev_priv)) {
> +		dp_tp_reg = TGL_DP_TP_CTL(pipe);
> +		trans_ddi_sel_port = TGL_TRANS_DDI_SELECT_PORT(port);
> +	}
>  
>  	trans_ddi_func_ctl_value = intel_de_read(dev_priv,
>  						 TRANS_DDI_FUNC_CTL(pipe));
>  	trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
>  	dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
> -
>  	trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> -				    TGL_TRANS_DDI_SELECT_PORT(port);
> +				    trans_ddi_sel_port;
>  	trans_conf_value |= PIPECONF_ENABLE;
>  	dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
>  
>  	intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
> -	intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> +	intel_de_write(dev_priv, dp_tp_reg, dp_tp_ctl_value);
>  	intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
>  		       trans_ddi_func_ctl_value);
>  }
> @@ -5557,6 +5575,7 @@ static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
>  static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	struct drm_i915_private *dev_priv = i915;
>  	u8 response = DP_TEST_NAK;
>  	u8 request = 0;
>  	int status;
> @@ -5582,6 +5601,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  		response = intel_dp_autotest_edid(intel_dp);
>  		break;
>  	case DP_TEST_LINK_PHY_TEST_PATTERN:
> +		if (!IS_ELKHARTLAKE(dev_priv) || !IS_TIGERLAKE(dev_priv)) {
> +			drm_dbg_kms(&i915->drm,
> +				"PHY compliance for platform not supported\n");
> +			return;
> +		}
>  		drm_dbg_kms(&i915->drm, "PHY_PATTERN test requested\n");
>  		response = intel_dp_autotest_phy_pattern(intel_dp);
>  		break;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx