Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp: Return the right vswing tables

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

On Fri, Mar 27, 2020 at 02:34:11PM -0700, José Roberto de Souza wrote:
> DDI ports have its encoders initialized with INTEL_OUTPUT_DDI type and
> later eDP ports that have the type changed to INTEL_OUTPUT_EDP.
> But for all other DDI ports it can drive HDMI or DP depending on what
> user connects to the ports.
> 
> ehl_get_combo_buf_trans() and tgl_get_combo_buf_trans() was checking
> for INTEL_OUTPUT_DP that was never true, causing eDP vswing tables
> being used.
> 
> So here changing the check to INTEL_OUTPUT_DDI, HDMI cases will be
> correctly handled as it do not use encoder->type, instead it calls the
> functions with INTEL_OUTPUT_HDMI as type parameter and HDMI don't have
> retraining.
> 
> Fixes: bd3cf6f7ce20 ("drm/i915/dp/tgl+: Update combo phy vswing tables")
> Cc: Clinton A Taylor <clinton.a.taylor@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 916a802af788..7af1572d4f1d 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -947,7 +947,7 @@ static const struct cnl_ddi_buf_trans *
>  ehl_get_combo_buf_trans(struct drm_i915_private *dev_priv, int type, int rate,
>  			int *n_entries)
>  {
> -	if (type == INTEL_OUTPUT_DP && rate > 270000) {
> +	if (type == INTEL_OUTPUT_DDI && rate > 270000) {

Please no. I'd rather not see "DDI" here. We want to check which mode
we're driving the output in, and "DDI" isn't one of the valid choices.

The fact that we sometimes pass in encoder->type is a bit of shortcut
to make the DP vs. EDP distinction. And so far every function knew to
only compare the value against EDP/HDMI and neve against DP. Looks like
someone broke that (admittedly crappy) convention.

We should probably fix this a bit higher up and make sure we only ever
pass in EDP/DP/HDMI, never DDI.

>  		*n_entries = ARRAY_SIZE(ehl_combo_phy_ddi_translations_hbr2_hbr3);
>  		return ehl_combo_phy_ddi_translations_hbr2_hbr3;
>  	}
> @@ -959,7 +959,7 @@ static const struct cnl_ddi_buf_trans *
>  tgl_get_combo_buf_trans(struct drm_i915_private *dev_priv, int type, int rate,
>  			int *n_entries)
>  {
> -	if (type != INTEL_OUTPUT_DP) {
> +	if (type != INTEL_OUTPUT_DDI) {
>  		return icl_get_combo_buf_trans(dev_priv, type, rate, n_entries);
>  	} else if (rate > 270000) {
>  		*n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> -- 
> 2.26.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