Re: [Intel-gfx] [PATCH] drm/i915: Stop programming DDI_BUF_TRANS_SELECT on recent platforms

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Dec 30, 20`19 at 03:45:41PM -0800, Matt Roper wrote:
> The bits in DDI_BUF_CTL related to DP vswing emphasis were removed on
> GLK since the relevant programming has moved to the PHY registers.  The
> bits still exist on BXT, but have a programming note indicating that
> they're ignored.  Let's stop programming them on gen9lp and and gen10+.
> 
> Bspec: 7534
> Bspec: 49533
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3a538789c585..c63a1712515c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1148,6 +1148,8 @@ void hsw_fdi_link_train(struct intel_encoder *encoder,
>  	/* Start the training iterating through available voltages and emphasis,
>  	 * testing each value twice. */
>  	for (i = 0; i < ARRAY_SIZE(hsw_ddi_translations_fdi) * 2; i++) {
> +		u32 tmp;
> +
>  		/* Configure DP_TP_CTL with auto-training */
>  		I915_WRITE(DP_TP_CTL(PORT_E),
>  					DP_TP_CTL_FDI_AUTOTRAIN |
> @@ -1159,10 +1161,10 @@ void hsw_fdi_link_train(struct intel_encoder *encoder,
>  		 * DDI E does not support port reversal, the functionality is
>  		 * achieved on the PCH side in FDI_RX_CTL, so no need to set the
>  		 * port reversal bit */
> -		I915_WRITE(DDI_BUF_CTL(PORT_E),
> -			   DDI_BUF_CTL_ENABLE |
> -			   ((crtc_state->fdi_lanes - 1) << 1) |
> -			   DDI_BUF_TRANS_SELECT(i / 2));
> +		tmp = DDI_BUF_CTL_ENABLE | ((crtc_state->fdi_lanes - 1) << 1);
> +		if (INTEL_GEN(dev_priv) <= 9 && !IS_GEN9_LP(dev_priv))

Only hsw/bdw have FDI so this part of the patch is pointless.

> +			tmp |= DDI_BUF_TRANS_SELECT(i / 2);
> +		I915_WRITE(DDI_BUF_CTL(PORT_E), tmp);
>  		POSTING_READ(DDI_BUF_CTL(PORT_E));
>  
>  		udelay(600);
> @@ -1238,13 +1240,16 @@ void hsw_fdi_link_train(struct intel_encoder *encoder,
>  
>  static void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	struct intel_digital_port *intel_dig_port =
>  		enc_to_dig_port(&encoder->base);
>  
> -	intel_dp->DP = intel_dig_port->saved_port_bits |
> -		DDI_BUF_CTL_ENABLE | DDI_BUF_TRANS_SELECT(0);
> +	intel_dp->DP = intel_dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE;
>  	intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count);
> +	if (INTEL_GEN(dev_priv) <= 9 && !IS_GEN9_LP(dev_priv))

gen < 9 || gen9_bc
would seem a bit easier on the brain.

> +		intel_dp->DP |= DDI_BUF_TRANS_SELECT(0);

That's just '|= 0' so perhaps a bit pointless. But I guess it does serve
as some sort of documentation. Though the whole use of
intel_ddi_init_dp_buf_reg() seems like confusing nonsesne. Especially
the tgl part as it doesn't even do what the comment suggests it should
be doing.

Anyways, last hunk is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  }
>  
>  static struct intel_encoder *
> -- 
> 2.23.0
> 
> _______________________________________________
> 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