On Wed, 2021-01-13 at 17:04 +0200, Imre Deak wrote:
> On Fri, Aug 21, 2020 at 11:48:37PM -0700, Khaled Almahallawy wrote:
> > Source needs to write DPCD 103-106 after receiving a PHY request to
> > change
> > swing/pre-emphasis after reading DPCD 206-207. This is especially
> > needed if
> > there is a retimer between source and sink and the retimer
> > implements AUX_CH
> > interception scheme to manage DP PHY settings (e.g. adjusting
> > Swing/Pre-emphasis
> > equalization level) for DP output channel . If the source doesn't
> > write to
> > DPCD 103-106, the retimer may not output the requested swing/pre-
> > emphasis and
> > eventually we fail compliance.
> >
> > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > 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. Also
Sure, I will rebase, test and resend.
> compliance.test_data.phytest has its own link params that may be
> different from the ones in crtc_state.
Actually, the request to change vswing/pre-emp and PHY test pattern
comes after the request to change Lane count and Link rate. Matter of
fact I think phytest.num_lanes and phytest.link_rate may be not
necessarily needed. However I cannot make this assertion yet until I
exhaust all automation options in the scope.
>
> 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
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
> There's been a discussion earlier that instead of open-coding here
> all
> the port enabling/disabling and link training programming sequences
> the
> driver's actual modesetting and link training code should be used
> instead, making those aware of the modified PHY test request link
> parameters.
Actually, Ville already did most of the work and I tested his code
intel_dp_phy_test
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6299,7 +6299,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
* FIXME get rid of the ad-hoc phy test modeset code
* and properly incorporate it into the normal modeset.
*/
- return false;
+ intel_dp_phy_test(encoder);
+ break;
}
return true;
This code even works with quirks of scripting mode of the scope as
well.
However, I am still struggling with scheduling a long HPD. I’m planning
to reply to Ville's patch he sent yesterday: “drm/i915: Fix the PHY
compliance test vs. hotplug mishap” to provide more info about test
automation.
Thank You for you feedback and review.
Khaled
> I suppose until that's done we could still merge this patch
> with the above intel_dp/crtc_state fix after you can confirm that
> this
> PHY test functionality actually works atm.
>
> --Imre
>
> > +
> > drm_dp_set_phy_test_pattern(&intel_dp->aux, data,
> > link_status[DP_DPCD_REV]);
> > }
> > --
> > 2.17.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
