[PATCH v2] drm/dp: follow DP link CTS spec to read link status back

[Lee, Shawn C]

On Wed, 07 Jul 2021, 4:14 p.m, Jani Nikula <jani.nikula at intel.com> wrote:
>On Wed, 07 Jul 2021, Lee Shawn C <shawn.c.lee at intel.com> wrote:
>> Refer to DP link CTS 1.2/1.4 spec, the following test case request 
>> source read DPCD 200h - 205h to get latest link status from sink.
>>
>> (4.3.2.4) Handling of IRQ HPD Pulse with No Error Status Bits Set
>> (400.3.2.1) Successful Link Re-training After IRQ HPD Pulse
>>             Due to Loss of Symbol Lock: HBR2 Extension
>> (400.3.2.2) Successful Link Re-training After IRQ HPD Pulse Due
>>             to Loss of Clock Recovery Lock: HBR2 Extension
>> (400.3.2.3) Successful Link Re-training After IRQ HPD Pulse Due
>>             to Loss of Inter-lane Alignment Lock: HBR2 Extension
>>
>> So far, DRM DP driver just read back the link status from 202h to 
>> 207h. DPR-120 would judge source can't pass these cases and shows 
>> below error messages.
>>
>> "Test FAILED, Source DUT does not read DPCD registers 200h-205h within 
>> 100 ms".
>
>Acked-by: Jani Nikula <jani.nikula at intel.com>
>
>for making the test pass iff everything else seems to work.
>
>The underlying question is, though, should we look at 0x200-0x201 for some status we don't look at?
>

Look into 200h. While doing link train with DPRX. In my opinion, sink_count and cp_ready status should be constant.
And sink would trigger HPD to source to notify 201h value was changed. Seems source driver don't need this value
at link training stage as well. What do you think?

Best regards,
Shawn

>
>>
>> v2: Use sizeof() to retrieve array size.
>>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Lyude Paul <lyude at redhat.com>
>> Cc: Simon Ser <contact at emersion.fr>
>> Cc: Cooper Chiou <cooper.chiou at intel.com>
>> Cc: William Tseng <william.tseng at intel.com>
>> Signed-off-by: Lee Shawn C <shawn.c.lee at intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c 
>> b/drivers/gpu/drm/drm_dp_helper.c index 24bbc710c825..4f03df317d62 
>> 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -410,17 +410,19 @@ int drm_dp_dpcd_read_phy_link_status(struct drm_dp_aux *aux,
>>  				     u8 link_status[DP_LINK_STATUS_SIZE])  {
>>  	int ret;
>> +	u8 full_link_stat[DP_LINK_STATUS_SIZE + 2];
>>  
>>  	if (dp_phy == DP_PHY_DPRX) {
>>  		ret = drm_dp_dpcd_read(aux,
>> -				       DP_LANE0_1_STATUS,
>> -				       link_status,
>> -				       DP_LINK_STATUS_SIZE);
>> +				       DP_SINK_COUNT,
>> +				       full_link_stat,
>> +				       sizeof(full_link_stat));
>>  
>>  		if (ret < 0)
>>  			return ret;
>>  
>> -		WARN_ON(ret != DP_LINK_STATUS_SIZE);
>> +		memcpy(link_status, full_link_stat + 2, DP_LINK_STATUS_SIZE);
>> +		WARN_ON(ret != DP_LINK_STATUS_SIZE + 2);
>>  
>>  		return 0;
>>  	}
>
>--
>Jani Nikula, Intel Open Source Graphics Center