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

[Souza, Jose]

On Mon, 2020-03-30 at 17:50 +0300, Ville Syrjälä wrote:
> 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 at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at 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.

Okay so for now I will just do the bellow:

if (type != INTEL_OUTPUT_EDP && type != INTEL_OUTPUT_HDMI && rate >
270000) {

Good enough for now?
> 
> >  		*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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx