On Tue, Jan 19, 2021 at 09:43:56AM +0200, Almahallawy, Khaled wrote: > > > [...] > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 79c27f91f42c..5044201ca742 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -5503,6 +5503,9 @@ void intel_dp_process_phy_request(struct > > > intel_dp *intel_dp) > > > > > > intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes); > > > > > > +drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, > > > +intel_dp->train_set, intel_dp- > > > >lane_count); > > > > This should be rebased on a recent change using instead > > crtc_state->lane_count. > > > That's also not completely correct since it's > > not guaranteed that the output is enabled (having up-to-date link > > params in crtc_state) at the time of this test request. Actually intel_dp_prep_phy_test() makes sure that the output is enabled, so nvm the above. > [...] > > I'm also not sure how intel_dp_autotest_phy_ddi_disable()/enable() > > affects the vswing/pre-emp setting of the source (DPTX) that got > > inited when the output was last enabled. The vs/pe programming > > sequence should be also part of the port enabling. Maybe the HW > > retains the config across the the above port disable/enable calls > > and so this happens not to be a problem. > > The requested Vswing/Pre-emph from test scope is coming as part of > short HPD not as part of Link Training, so I’m not sure how we can use > these requested vswing/pre-emph values as we do for lane count and Link > rate as in : intel_dp_adjust_compliance_config Looks like during PHY testing a regular link training should be performed (including any LTTPRs on the link), and then for DPRX instead of the regular cr/eq just set the requested vs/pe levels and the test pattern. If TEST_LANE_COUNT/RATE changes the link needs to be retrained again, if only the requested test pattern or vs/pe levels change then changing only these w/o retraining the link should be ok. > However the rationale behind > intel_dp_autotest_phy_ddi_disable()/enable() is based on Specs:50482 > which said TRANS_CONF and TRANS_DDI_FUNC_CTL must be disabled prior to > enabling the test pattern Ok, makes sense, so this indeed seems to need special casing for PHY testing. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
