[RFC 03/13] drm/dp_helper: Add FRL training support for a DP-HDMI2.1 PCON

Nautiyal, Ankit K ankit.k.nautiyal at intel.com
Sun Nov 1 05:53:05 UTC 2020


On 10/19/2020 3:03 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 03/13] drm/dp_helper: Add FRL training support for a DP-HDMI2.1
>> PCON
> You can name this patch as "Add Helpers for FRL Link Training support for DP-HDMI2.1 PCON"

Makes sense, will change the commit msg in the next version.


>
>> This patch adds support for configuring a PCON device, connected as a DP
>> branched device to enable FRL Link training with a HDMI2.1 + sink.
>>
>> v2: Minor changes:
>> -removed unnecessary argument supplied to a drm helper function.
>> -fixed return value for max frl read from pcon.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_helper.c | 305 ++++++++++++++++++++++++++++++++
>>   include/drm/drm_dp_helper.h     |  80 +++++++++
>>   2 files changed, 385 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c index 14ddf28ecac0..df858533dbf7 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -2591,3 +2591,308 @@ void drm_dp_vsc_sdp_log(const char *level, struct
>> device *dev,  #undef DP_SDP_LOG  }  EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>> +
>> +/**
>> + * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
>> + * @dpcd: DisplayPort configuration data
>> + * @port_cap: port capabilities
>> + *
>> + * Returns maximum frl bandwidth supported by PCON in GBPS,
>> + * returns 0 if not supported.
>> + **/
>> +int drm_dp_get_pcon_max_frl_bw(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>> +       const u8 port_cap[4])
>> +{
>> +int bw;
>> +u8 buf;
>> +
>> +buf = port_cap[2];
>> +bw = buf & DP_PCON_MAX_FRL_BW;
>> +
>> +switch (bw) {
>> +case DP_PCON_MAX_9GBPS:
>> +return 9;
>> +case DP_PCON_MAX_18GBPS:
>> +return 18;
>> +case DP_PCON_MAX_24GBPS:
>> +return 24;
>> +case DP_PCON_MAX_32GBPS:
>> +return 32;
>> +case DP_PCON_MAX_40GBPS:
>> +return 40;
>> +case DP_PCON_MAX_48GBPS:
>> +return 48;
>> +case DP_PCON_MAX_0GBPS:
>> +default:
>> +return 0;
>> +}
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_get_pcon_max_frl_bw);
>> +
>> +/**
>> + * drm_dp_get_hdmi_max_frl_bw() - maximum frl supported by HDMI Sink
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns maximum frl bandwidth supported by HDMI in Gbps on success,
>> + * returns 0, if not supported.
>> + **/
>> +int drm_dp_get_hdmi_max_frl_bw(struct drm_dp_aux *aux) {
> s/hdmi/hdmi_sink/ will make it clear that we are referring to sink here.


Agreed. will change the name as suggested in next version.

>
>> +u8 buf;
>> +int bw, ret;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_SINK, &buf);
>> +if (ret < 0)
>> +return 0;
>> +bw = buf & DP_HDMI_SINK_LINK_BW;
>> +
>> +switch (bw) {
>> +case DP_HDMI_SINK_BW_9GBPS:
>> +return 9;
>> +case DP_HDMI_SINK_BW_18GBPS:
>> +return 18;
>> +case DP_HDMI_SINK_BW_24GBPS:
>> +return 24;
>> +case DP_HDMI_SINK_BW_32GBPS:
>> +return 32;
>> +case DP_HDMI_SINK_BW_40GBPS:
>> +return 40;
>> +case DP_HDMI_SINK_BW_48GBPS:
>> +return 48;
>> +case DP_HDMI_SINK_BW_0GBPS:
>> +default:
>> +return 0;
>> +}
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_get_hdmi_max_frl_bw);
>> +
>> +/**
>> + * drm_dp_pcon_frl_prepare() - Prepare PCON for FRL.
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns 0 if success, else returns negative error code.
>> + **/
>> +int drm_dp_pcon_frl_prepare(struct drm_dp_aux *aux, bool
>> +enable_frl_ready_hpd) {
>> +int ret;
>> +u8 buf = DP_PCON_ENABLE_SOURCE_CTL_MODE |
>> + DP_PCON_ENABLE_LINK_FRL_MODE;
>> +
>> +if (enable_frl_ready_hpd)
>> +buf |= DP_PCON_ENABLE_HPD_READY;
>> +
>> +ret = drm_dp_dpcd_writeb(aux, DP_PCON_HDMI_LINK_CONFIG_1, buf);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_frl_prepare);
>> +
>> +/**
>> + * drm_dp_pcon_is_frl_ready() - Is PCON ready for FRL
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns true if success, else returns false.
>> + **/
>> +bool drm_dp_pcon_is_frl_ready(struct drm_dp_aux *aux) {
>> +int ret;
>> +u8 buf;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_TX_LINK_STATUS, &buf);
>> +if (ret < 0)
>> +return false;
>> +
>> +if (buf & DP_PCON_FRL_READY)
>> +return true;
>> +
>> +return false;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_is_frl_ready);
>> +
>> +/**
>> + * drm_dp_pcon_frl_configure_1() - Set HDMI LINK Configuration-Step1
>> + * @aux: DisplayPort AUX channel
>> + * max_frl_mask: mask for selecting the bandwidths supported by source,
> @missing from variable and its not a mask here but an absolute value.


You are right, I will fix this in the next version.

>
>> + * to be tried by Pcon f/w.
>> + * @concurrent_mode: true if concurrent mode or operation is required,
>> + * false otherwise.
>> + *
>> + * Returns 0 if success, else returns negative error code.
>> + **/
>> +
>> +int drm_dp_pcon_frl_configure_1(struct drm_dp_aux *aux, int max_frl_gbps,
>> +bool concurrent_mode)
>> +{
>> +int ret;
>> +u8 buf;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_LINK_CONFIG_1, &buf);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (concurrent_mode)
>> +buf |= DP_PCON_ENABLE_CONCURRENT_LINK;
>> +else
>> +buf &= ~DP_PCON_ENABLE_CONCURRENT_LINK;
>> +
>> +switch (max_frl_gbps) {
>> +case 9:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_9GBPS;
>> +break;
>> +case 18:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_18GBPS;
>> +break;
>> +case 24:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_24GBPS;
>> +break;
>> +case 32:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_32GBPS;
>> +break;
>> +case 40:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_40GBPS;
>> +break;
>> +case 48:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_48GBPS;
>> +break;
>> +case 0:
>> +buf |=  DP_PCON_ENABLE_MAX_BW_0GBPS;
>> +break;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +ret = drm_dp_dpcd_writeb(aux, DP_PCON_HDMI_LINK_CONFIG_1, buf);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_frl_configure_1);
>> +
>> +/**
>> + * drm_dp_pcon_frl_configure_2() - Set HDMI Link configuration Step-2
>> + * @aux: DisplayPort AUX channel
>> + * @max_frl_mask : Max FRL BW to be tried by the PCON with HDMI Sink
>> + * @extended_train_mode : true for Extended Mode, false for Normal Mode.
>> + * In Normal mode, the PCON tries each frl bw from the max_frl_mask
>> +starting
>> + * from min, and stops when link training is successful. In Extended
>> +mode, all
>> + * frl bw selected in the mask are trained by the PCON.
>> + *
>> + * Returns 0 if success, else returns negative error code.
>> + **/
>> +int drm_dp_pcon_frl_configure_2(struct drm_dp_aux *aux, int max_frl_mask,
>> +bool extended_train_mode)
>> +{
>> +int ret;
>> +u8 buf = 0;
>> +
>> +buf |= max_frl_mask;
> Can just initialize buf to max _frl_mask.


Agreed.

>
>> +
>> +if (extended_train_mode)
>> +buf |= DP_PCON_FRL_LINK_TRAIN_EXTENDED;
>> +
>> +ret = drm_dp_dpcd_writeb(aux, DP_PCON_HDMI_LINK_CONFIG_2, buf);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_frl_configure_2);
>> +
>> +/**
>> + * drm_dp_pcon_reset_frl_config() - Re-Set HDMI Link configuration.
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns 0 if success, else returns negative error code.
>> + **/
>> +int drm_dp_pcon_reset_frl_config(struct drm_dp_aux *aux) {
>> +int ret;
>> +
>> +ret = drm_dp_dpcd_writeb(aux, DP_PCON_HDMI_LINK_CONFIG_1, 0x0);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_reset_frl_config);
>> +
>> +/**
>> + * drm_dp_pcon_frl_enable() - Enable HDMI link through FRL
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns 0 if success, else returns negative error code.
>> + **/
>> +int drm_dp_pcon_frl_enable(struct drm_dp_aux *aux) {
>> +int ret;
>> +u8 buf = 0;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_LINK_CONFIG_1, &buf);
>> +if (ret < 0)
>> +return ret;
>> +if (!(buf & DP_PCON_ENABLE_SOURCE_CTL_MODE)) {
>> +DRM_DEBUG_KMS("PCON in Autonomous mode, can't enable
>> FRL\n");
>> +return -EINVAL;
>> +}
>> +buf |= DP_PCON_ENABLE_HDMI_LINK;
>> +ret = drm_dp_dpcd_writeb(aux, DP_PCON_HDMI_LINK_CONFIG_1, buf);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_frl_enable);
>> +
>> +/**
>> + * drm_dp_pcon_hdmi_link_active() - check if the PCON HDMI LINK status is
>> active.
>> + * @aux: DisplayPort AUX channel
>> + *
>> + * Returns true if link is active else returns false.
>> + **/
>> +bool drm_dp_pcon_hdmi_link_active(struct drm_dp_aux *aux) {
>> +u8 buf;
>> +int ret;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_TX_LINK_STATUS, &buf);
>> +if (ret < 0)
>> +return false;
>> +
>> +return buf & DP_PCON_HDMI_TX_LINK_ACTIVE; }
>> +EXPORT_SYMBOL(drm_dp_pcon_hdmi_link_active);
>> +
>> +/**
>> + * drm_dp_pcon_hdmi_link_mode() - get the PCON HDMI LINK MODE
>> + * @aux: DisplayPort AUX channel
>> + * @frl_trained_mask: pointer to store bitmask of the trained bw configuration.
>> + * Valid only if the MODE returned is FRL. For Normal Link training
>> +mode
>> + * only 1 of the bits will be set, but in case of Extended mode, more
>> +than
>> + * one bits can be set.
>> + *
>> + * Returns the link mode : TMDS or FRL on success, else retunes
> Typo in returns

Will fix in the next version.


>
>> +negative error
>> + * code.
>> + **/
>> +int drm_dp_pcon_hdmi_link_mode(struct drm_dp_aux *aux, u8
>> +*frl_trained_mask) {
>> +u8 buf;
>> +int mode;
>> +int ret;
>> +
>> +ret = drm_dp_dpcd_readb(aux, DP_PCON_HDMI_POST_FRL_STATUS,
>> &buf);
>> +if (ret < 0)
>> +return ret;
>> +
>> +mode = buf & DP_PCON_HDMI_LINK_MODE;
>> +
>> +if (frl_trained_mask && DP_PCON_HDMI_MODE_FRL == mode)
> Should be bitwise and not logical and right ?. Also this mask is a bit ambigious,
> we could just use absolute bandwidth which would be more clear.

Actually these are two conditions here. First condition is to check if 
the frl_trained_mask is not a NULL pointer.

Second is to check if the FRL mode Hdmi Link is in FRL mode.

We need to set the FRL link training mask, only if the HDMI link is in 
FRL mode.

>
>> +*frl_trained_mask = (buf & DP_PCON_HDMI_FRL_TRAINED_BW)
>>>> 1;
>> +
>> +return mode;
>> +}
>> +EXPORT_SYMBOL(drm_dp_pcon_hdmi_link_mode);
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
>> f55a9d1320ca..d6f79b2d1287 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -411,6 +411,17 @@ struct drm_device;
>>   # define DP_DS_10BPC            1
>>   # define DP_DS_12BPC            2
>>   # define DP_DS_16BPC            3
>> +/* HDMI2.1 PCON FRL CONFIGURATION */
>> +# define DP_PCON_MAX_FRL_BW                 (7 << 2)
>> +# define DP_PCON_MAX_0GBPS                  (0 << 2)
>> +# define DP_PCON_MAX_9GBPS                  (1 << 2)
>> +# define DP_PCON_MAX_18GBPS                 (2 << 2)
>> +# define DP_PCON_MAX_24GBPS                 (3 << 2)
>> +# define DP_PCON_MAX_32GBPS                 (4 << 2)
>> +# define DP_PCON_MAX_40GBPS                 (5 << 2)
>> +# define DP_PCON_MAX_48GBPS                 (6 << 2)
>> +# define DP_PCON_SOURCE_CTL_MODE            (1 << 5)
>> +
>>   /* offset 3 for DVI */
>>   # define DP_DS_DVI_DUAL_LINK    (1 << 1)
>>   # define DP_DS_DVI_HIGH_COLOR_DEPTH    (1 << 2)
>> @@ -1053,6 +1064,61 @@ struct drm_device;
>>   #define DP_CEC_RX_MESSAGE_BUFFER               0x3010
>>   #define DP_CEC_TX_MESSAGE_BUFFER               0x3020
>>   #define DP_CEC_MESSAGE_BUFFER_LENGTH             0x10
>> +/* PROTOCOL CONVERSION HDMI SINK */
>> +#define DP_PCON_HDMI_SINK                      0x3035
>> +# define DP_HDMI_SINK_LINK_BW                  (7 << 0)
>> +# define DP_HDMI_SINK_BW_0GBPS       0
>> +# define DP_HDMI_SINK_BW_9GBPS       1
>> +# define DP_HDMI_SINK_BW_18GBPS       2
>> +# define DP_HDMI_SINK_BW_24GBPS       3
>> +# define DP_HDMI_SINK_BW_32GBPS       4
>> +# define DP_HDMI_SINK_BW_40GBPS       5
>> +# define DP_HDMI_SINK_BW_48GBPS       6
>> +
>> +/* PCON CONFIGURE-1 FRL FOR HDMI SINK */
>> +#define DP_PCON_HDMI_LINK_CONFIG_1             0x305A
>> +# define DP_PCON_ENABLE_MAX_FRL_BW             (7 << 0)
>> +# define DP_PCON_ENABLE_MAX_BW_0GBPS       0
>> +# define DP_PCON_ENABLE_MAX_BW_9GBPS       1
>> +# define DP_PCON_ENABLE_MAX_BW_18GBPS       2
>> +# define DP_PCON_ENABLE_MAX_BW_24GBPS       3
>> +# define DP_PCON_ENABLE_MAX_BW_32GBPS       4
>> +# define DP_PCON_ENABLE_MAX_BW_40GBPS       5
>> +# define DP_PCON_ENABLE_MAX_BW_48GBPS       6
>> +# define DP_PCON_ENABLE_SOURCE_CTL_MODE       (1 << 3)
>> +# define DP_PCON_ENABLE_CONCURRENT_LINK       (1 << 4)
>> +# define DP_PCON_ENABLE_LINK_FRL_MODE         (1 << 5)
>> +# define DP_PCON_ENABLE_HPD_READY      (1 << 6)
>> +# define DP_PCON_ENABLE_HDMI_LINK             (1 << 7)
>> +
>> +/* PCON CONFIGURE-2 FRL FOR HDMI SINK */
>> +#define DP_PCON_HDMI_LINK_CONFIG_2            0x305B
>> +# define DP_PCON_MAX_LINK_BW_MASK             (0x3F << 0)
>> +# define DP_PCON_FRL_BW_MASK_9GBPS            (1 << 0)
>> +# define DP_PCON_FRL_BW_MASK_18GBPS           (1 << 1)
>> +# define DP_PCON_FRL_BW_MASK_24GBPS           (1 << 2)
>> +# define DP_PCON_FRL_BW_MASK_32GBPS           (1 << 3)
>> +# define DP_PCON_FRL_BW_MASK_40GBPS           (1 << 4)
>> +# define DP_PCON_FRL_BW_MASK_48GBPS           (1 << 5)
>> +# define DP_PCON_FRL_LINK_TRAIN_EXTENDED      (1 << 6)
>> +
>> +/* PCON HDMI LINK STATUS */
>> +#define DP_PCON_HDMI_TX_LINK_STATUS           0x303B
>> +# define DP_PCON_HDMI_TX_LINK_ACTIVE          (1 << 0)
>> +# define DP_PCON_FRL_READY      (1 << 1)
>> +
>> +/* PCON HDMI POST FRL STATUS */
>> +#define DP_PCON_HDMI_POST_FRL_STATUS          0x3036
>> +# define DP_PCON_HDMI_LINK_MODE               (1 << 0)
>> +# define DP_PCON_HDMI_MODE_TMDS               0
>> +# define DP_PCON_HDMI_MODE_FRL                1
>> +# define DP_PCON_HDMI_FRL_TRAINED_BW          (0x3F << 1)
> This should be 0x3 < 1
>
>> +# define DP_PCON_FRL_TRAINED_BW_9GBPS      (1 << 1)
>> +# define DP_PCON_FRL_TRAINED_BW_18GBPS      (1 << 2)
>> +# define DP_PCON_FRL_TRAINED_BW_24GBPS      (1 << 3)
>> +# define DP_PCON_FRL_TRAINED_BW_32GBPS      (1 << 4)
>> +# define DP_PCON_FRL_TRAINED_BW_40GBPS      (1 << 5)
>> +# define DP_PCON_FRL_TRAINED_BW_48GBPS      (1 << 6)
>>
>>   #define DP_PROTOCOL_CONVERTER_CONTROL_00x3050 /* DP 1.3
>> */
>>   # define DP_HDMI_DVI_OUTPUT_CONFIG(1 << 0) /* DP 1.3 */
>> @@ -1967,4 +2033,18 @@ int drm_dp_get_phy_test_pattern(struct drm_dp_aux
>> *aux,
>>   struct drm_dp_phy_test_params *data);  int
>> drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
>>   struct drm_dp_phy_test_params *data, u8
>> dp_rev);
>> +int drm_dp_get_pcon_max_frl_bw(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>> +       const u8 port_cap[4]);
>> +int drm_dp_get_hdmi_max_frl_bw(struct drm_dp_aux *aux); int
>> +drm_dp_pcon_frl_prepare(struct drm_dp_aux *aux, bool
>> +enable_frl_ready_hpd); bool drm_dp_pcon_is_frl_ready(struct drm_dp_aux
>> +*aux); int drm_dp_pcon_frl_configure_1(struct drm_dp_aux *aux, int
>> max_frl_gbps,
>> +bool concurrent_mode);
>> +int drm_dp_pcon_frl_configure_2(struct drm_dp_aux *aux, int max_frl_mask,
>> +bool extended_train_mode);
>> +int drm_dp_pcon_reset_frl_config(struct drm_dp_aux *aux); 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);
> Leave a blank line here.

Agreed. Will take care in the next version.


Thanks & Regards,

Ankit

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


More information about the dri-devel mailing list