Re: [Intel-gfx] [PATCH] drm/i915/dp: DPTX writes Swing/Pre-emphs(DPCD 0x103-0x106) requested during PHY Layer testing.

["Almahallawy, Khaled" <khaled.almahallawy@intel.com>]

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@intel.com>
> > ---
> >  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx