Re: [Intel-gfx] [PATCH] drm/display: Don't rewrite link config when setting phy test pattern

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 15 Sep 2022, Khaled Almahallawy <khaled.almahallawy@intel.com> wrote:
> The sequence for Source DP PHY CTS automation is [2][1]:
> 1- Emulate successful Link Training(LT)
> 2- Short HPD and change link rates and number of lanes by LT.
> (This is same flow for Link Layer CTS)
> 3- Short HPD and change PHY test pattern and swing/pre-emphasis
> levels (This step should not trigger LT)
>
> The problem is with DP PHY compliance setup as follow:
>
>      [DPTX + on board LTTPR]------Main Link--->[Scope]
>      	     	        ^                         |
> 			|                         |
> 			|                         |
> 			----------Aux Ch------>[Aux Emulator]
>
> At step 3, before writing TRAINING_LANEx_SET/LINK_QUAL_PATTERN_SET
> to declare the pattern/swing requested by scope, we write link
> config in LINK_BW_SET/LANE_COUNT_SET on a port that has LTTPR.
> As LTTPR snoops aux transaction, LINK_BW_SET/LANE_COUNT_SET writes
> indicate a LT will start [Check DP 2.0 E11 -Sec 3.6.8.2 & 3.6.8.6.3],
> and LTTPR will reset the link and stop sending DP signals to
> DPTX/Scope causing the measurements to fail. Note that step 3 will
> not trigger LT and DP link will never recovered by the
> Aux Emulator/Scope.
>
> The reset of link can be tested with a monitor connected to LTTPR
> port simply by writing to LINK_BW_SET or LANE_COUNT_SET as follow
>
>   igt/tools/dpcd_reg write --offset=0x100 --value 0x14 --device=2
>
> OR
>
>   printf '\x14' | sudo dd of=/dev/drm_dp_aux2 bs=1 count=1 conv=notrunc
>   seek=$((0x100))
>
> This single aux write causes the screen to blank, sending short HPD to
> DPTX, setting LINK_STATUS_UPDATE = 1 in DPCD 0x204, and triggering LT.
>
> As stated in [1]:
> "Before any TX electrical testing can be performed, the link between a
> DPTX and DPRX (in this case, a piece of test equipment), including all
> LTTPRs within the path, shall be trained as defined in this Standard."
>
> In addition, changing Phy pattern/Swing/Pre-emphasis (Step 3) uses the
> same link rate and lane count applied on step 2, so no need to redo LT.
>
> The fix is to not rewrite link config in step 3, and just writes
> TRAINING_LANEx_SET and LINK_QUAL_PATTERN_SET
>
> [1]: DP 2.0 E11 - 3.6.11.1 LTTPR DPTX_PHY Electrical Compliance
>
> [2]: Configuring UnigrafDPTC Controller - Automation Test Sequence
> https://www.keysight.com/us/en/assets/9922-01244/help-files/
> D9040DPPC-DisplayPort-Test-Software-Online-Help-latest.chm
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Or Cochvi <or.cochvi@intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>

Pushed to drm-misc-next, thanks for the patch.

I didn't seek further confirmation because i915 is still the only user
of this function it seems.

BR,
Jani.


> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index 92990a3d577a..9f055d9710ea 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2670,17 +2670,8 @@ int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
>  				struct drm_dp_phy_test_params *data, u8 dp_rev)
>  {
>  	int err, i;
> -	u8 link_config[2];
>  	u8 test_pattern;
>  
> -	link_config[0] = drm_dp_link_rate_to_bw_code(data->link_rate);
> -	link_config[1] = data->num_lanes;
> -	if (data->enhanced_frame_cap)
> -		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> -	err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, link_config, 2);
> -	if (err < 0)
> -		return err;
> -
>  	test_pattern = data->phy_pattern;
>  	if (dp_rev < 0x12) {
>  		test_pattern = (test_pattern << 2) &

-- 
Jani Nikula, Intel Open Source Graphics Center