On Thu, Sep 30, 2021 at 07:19:06PM +0300, Imre Deak wrote: > On Thu, Sep 30, 2021 at 07:16:10PM +0300, Ville Syrjälä wrote: > > On Thu, Sep 30, 2021 at 07:09:03PM +0300, Imre Deak wrote: > > > On Thu, Sep 30, 2021 at 04:43:09PM +0300, Ville Syrjala wrote: > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > Setting DP_PORT_EN in intel_dp->DP is already handled by > > > > intel_dp_enable_port() so there is no point in setting it also > > > > from the link training code. > > > > > > > > For DDI platforms a bit with that name doesn't even exist. The > > > > counterpart is DDI_BUF_CTL_ENABLE, which is already set up by > > > > intel_ddi_prepare_link_retrain(). Fortunately it is the same bit > > > > so there was no harm in doing this from the platform independent > > > > code as well. But it's just confusing when platform independent > > > > code sets platform specific bits in intel_dp->DP. Just get rid > > > > of it. > > > > > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > Reviewed-by: Imre Deak <imre.deak.intel.com> > > > > > > On pre-DDI platforms intel_dp_enable_port() may not be called before > > > short HPD/link-retraining, but the init/resume time HW readout will > > > set DP_PORT_EN for that case. > > > > I actually wonder what happens on DDI platforms. We don't do the resume > > readout there AFAICS, so if link retraining happens before any modesets > > are we just screwed atm? > > Yea. It's unlikely to cause a problem though, since resume mostly happens with disabled outputs? > > > > > > > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_link_training.c | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > > > index 449499a5c4c1..053ed9302cda 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > > > @@ -499,8 +499,6 @@ intel_dp_prepare_link_train(struct intel_dp *intel_dp, > > > > DP_SET_ANSI_128B132B : DP_SET_ANSI_8B10B; > > > > drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2); > > > > > > > > - intel_dp->DP |= DP_PORT_EN; > > > > - > > > > return true; > > > > } > > > > > > > > -- > > > > 2.32.0 > > > > > > > > -- > > Ville Syrjälä > > Intel