[RFC 07/13] drm/dp_helper: Add support for link status and link recovery

Sun Nov 1 06:18:31 UTC 2020

On 10/19/2020 4:07 AM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
>> Sent: Thursday, October 15, 2020 4:23 PM
>> To: intel-gfx at lists.freedesktop.org
>> Cc: dri-devel at lists.freedesktop.org; Shankar, Uma <uma.shankar at intel.com>;
>> Kulkarni, Vandita <vandita.kulkarni at intel.com>; ville.syrjala at linux.intel.com;
>> Sharma, Swati2 <swati2.sharma at intel.com>
>> Subject: [RFC 07/13] drm/dp_helper: Add support for link status and link
>> recovery
> Move this in the start of the series along with rest of the generic DRM helpers.
Alight, will move this along with other DRM helpers.
>
>> From: Swati Sharma <swati2.sharma at intel.com>
>>
>> This patch adds support for link status and link recovery. There are specific
>> DPCD’s defined for link status check and recovery in case of any issues. PCON will
>> communicate the same using an IRQ_HPD to source. HDMI sink would have
>> indicated the same to PCON using SCDC interrupt mechanism. While source can
>> always read final HDMI sink’s status using I2C over AUX, it’s easier and faster to
>> read the PCON’s already read HDMI sink’s status registers.
>>
>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_helper.c | 33 +++++++++++++++++++++++++++++++++
>>   include/drm/drm_dp_helper.h     | 16 ++++++++++++++++
>>   2 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c index df858533dbf7..33a4ac2fb225 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -2896,3 +2896,36 @@ int drm_dp_pcon_hdmi_link_mode(struct
>> drm_dp_aux *aux, u8 *frl_trained_mask)
>>   return mode;
>>   }
>>   EXPORT_SYMBOL(drm_dp_pcon_hdmi_link_mode);
>> +
>> +void drm_dp_pcon_hdmi_frl_link_error_count(struct drm_dp_aux *aux,
>> +   struct drm_connector *connector) {
> This just prints a message if error counts are detected. There isn't any recovery here.
> May be you should re-phrase the patch header and description to reflect the same.
>
> Also what will be the usage of this just prints a message, may be return the error to caller
> to plan a recovery or link reset .

You are right, this patch is just adding few DPCDs that will be useful 
in detection of Link failure.

These registers will be used while servicing short pulse IRQ_HPD and 
there the FRL training will be restarted. In case of failure, will be 
switched to TMDS mode.

This is taken care in the subsequent patch for i915, which modifies the 
intel_dp_short_pulse().

The helper function is more for debugging purpose, to identify the exact 
lanes which had issues and the magnitude of the error counts in that lane.

I will rephrase the commit message to be more clear.

>> +u8 buf, error_count;
>> +int i, num_error;
>> +struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>> +
>> +for (i = 0; i < hdmi->max_lanes; i++)
>> +{
>> +if (drm_dp_dpcd_readb(aux,
>> DP_PCON_HDMI_ERROR_STATUS_LN0 + i , &buf) < 0)
>> +return;
>> +
>> +error_count = buf & DP_PCON_HDMI_ERROR_COUNT_MASK;
>> +
>> +switch(error_count) {
> Alignment is off.

Agreed. Will fix this in next version.

>
>> +case DP_PCON_HDMI_ERROR_COUNT_HUNDRED_PLUS:
>> +num_error = 100;
>> +break;
>> +case DP_PCON_HDMI_ERROR_COUNT_TEN_PLUS:
>> +num_error = 10;
>> +break;
>> +case DP_PCON_HDMI_ERROR_COUNT_THREE_PLUS:
>> +num_error = 3;
>> +break;
>> +default:
>> +num_error = 0;
>> +}
>> +
>> +DRM_ERROR("More than %d errors since the last read for lane
>> %d", num_error, i);
>> +}
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_hdmi_frl_link_error_count);
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
>> d6f79b2d1287..eb26c86dc8ca 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -946,6 +946,11 @@ struct drm_device;
>>   # define DP_CEC_IRQ                          (1 << 2)
>>
>>   #define DP_LINK_SERVICE_IRQ_VECTOR_ESI0     0x2005   /* 1.2 */
>> +# define RX_CAP_CHANGED                      (1 << 0)
>> +# define LINK_STATUS_CHANGED                 (1 << 1)
>> +# define STREAM_STATUS_CHANGED               (1 << 2)
>> +# define HDMI_LINK_STATUS_CHANGED            (1 << 3)
>> +# define CONNECTED_OFF_ENTRY_REQUESTED       (1 << 4)
>>
>>   #define DP_PSR_ERROR_STATUS                 0x2006  /* XXX 1.2? */
>>   # define DP_PSR_LINK_CRC_ERROR              (1 << 0)
>> @@ -1130,6 +1135,16 @@ struct drm_device;
>>   #define DP_PROTOCOL_CONVERTER_CONTROL_20x3052 /* DP 1.3
>> */
>>   # define DP_CONVERSION_TO_YCBCR422_ENABLE(1 << 0) /* DP 1.3 */
>>
>> +/* PCON Downstream HDMI ERROR Status per Lane */
>> +#define DP_PCON_HDMI_ERROR_STATUS_LN0          0x3037
>> +#define DP_PCON_HDMI_ERROR_STATUS_LN1          0x3038
>> +#define DP_PCON_HDMI_ERROR_STATUS_LN2          0x3039
>> +#define DP_PCON_HDMI_ERROR_STATUS_LN3          0x303A
>> +# define DP_PCON_HDMI_ERROR_COUNT_MASK         (0x7 << 0)
>> +# define DP_PCON_HDMI_ERROR_COUNT_THREE_PLUS   (1 << 0)
>> +# define DP_PCON_HDMI_ERROR_COUNT_TEN_PLUS     (1 << 1)
>> +# define DP_PCON_HDMI_ERROR_COUNT_HUNDRED_PLUS (1 << 2)
>> +
>>   /* HDCP 1.3 and HDCP 2.2 */
>>   #define DP_AUX_HDCP_BKSV0x68000
>>   #define DP_AUX_HDCP_RI_PRIME0x68005
>> @@ -2047,4 +2062,5 @@ int drm_dp_pcon_frl_enable(struct drm_dp_aux *aux);
>>
>>   bool drm_dp_pcon_hdmi_link_active(struct drm_dp_aux *aux);  int
>> drm_dp_pcon_hdmi_link_mode(struct drm_dp_aux *aux, u8 *frl_trained_mask);
>> +void drm_dp_pcon_hdmi_frl_link_error_count(struct drm_dp_aux *aux,
>> +struct drm_connector *connector);
> Leave a blank line.

Agreed. Will fix this in next version.

Thanks & Regards,

Ankit

>
>>   #endif /* _DRM_DP_HELPER_H_ */
>> --
>> 2.17.1