On Wed, May 09, 2018 at 06:22:44PM +0800, Lin Huang wrote:
> DP firmware uses fixed phy config values to do training, but some
> boards need to adjust these values to fit for their unique hardware
> design. So if the phy is using custom config values, do software
> link training instead of relying on firmware, if software training
> fail, keep firmware training as a fallback if sw training fails.
> 
> Signed-off-by: Chris Zhong <z...@rock-chips.com>
> Signed-off-by: Lin Huang <h...@rock-chips.com>

FTR, I've previously reviewed this patch at
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/985573

> ---
> Changes in v2:
> - update patch following Enric suggest
> 
>  drivers/gpu/drm/rockchip/Makefile               |   3 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          |  24 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.h          |   2 +
>  drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 391 
> ++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/cdn-dp-reg.c           |  34 ++-
>  drivers/gpu/drm/rockchip/cdn-dp-reg.h           |  38 ++-
>  6 files changed, 479 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> 

/snip

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h 
> b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 46159b2..c6050ab 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -84,6 +84,7 @@ struct cdn_dp_device {
>       bool connected;
>       bool active;
>       bool suspended;
> +     bool sw_training_success;

Can you change this to use_fw_training instead? So it will be false if sw
training succeeds, and true if sw training fails and needs to fallback to fw.

>  
>       const struct firmware *fw;      /* cdn dp firmware */
>       unsigned int fw_version;        /* cdn fw version */
> @@ -106,6 +107,7 @@ struct cdn_dp_device {
>       u8 ports;
>       u8 lanes;
>       int active_port;
> +     u8 train_set[4];
>  
>       u8 dpcd[DP_RECEIVER_CAP_SIZE];
>       bool sink_has_audio;

/snip

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c 
> b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index afdfda0..bd0aed5 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -17,7 +17,9 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
>  #include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>  
>  #include "cdn-dp-core.h"
>  #include "cdn-dp-reg.h"
> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device *dp, 
> u8 module_id,
>       return 0;
>  }
>  
> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>  {
>       u8 msg[6];
>  
> @@ -606,7 +608,35 @@ static int cdn_dp_get_training_status(struct 
> cdn_dp_device *dp)
>  int cdn_dp_train_link(struct cdn_dp_device *dp)
>  {
>       int ret;
> +     struct cdn_dp_port *port = dp->port[dp->active_port];
> +     struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>  
> +     /*
> +      * DP firmware uses fixed phy config values to do training, but some
> +      * boards need to adjust these values to fit for their unique hardware
> +      * design. So if the phy is using custom config values, do software
> +      * link training instead of relying on firmware, if software training
> +      * fail, keep firmware training as a fallback if sw training fails.
> +      */
> +     if (tcphy->need_software_training) {

As discussed in the downstream review, I'd rather default to sw training and
fallback to fw training if we must.

> +             ret = cdn_dp_software_train_link(dp);
> +             if (ret) {
> +                     DRM_DEV_ERROR(dp->dev,
> +                             "Failed to do software training %d\n", ret);
> +                     goto do_fw_training;
> +             }
> +             ret = cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
> +             if (ret) {
> +                     DRM_DEV_ERROR(dp->dev,
> +                     "Failed to write SOURCE_HDTX_CAR register %d\n", ret);
> +                     goto do_fw_training;
> +             }
> +             dp->sw_training_success = true;
> +             return 0;
> +     }
> +
> +do_fw_training:
> +     dp->sw_training_success = false;

Can you please add a DRM_DEBUG_KMS log message here?

>       ret = cdn_dp_training_start(dp);
>       if (ret) {
>               DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n", ret);
> @@ -621,7 +651,7 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>  
>       DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n", dp->link.rate,
>                         dp->link.num_lanes);
> -     return ret;
> +     return 0;
>  }
>  
>  int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h 
> b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index 6580b11..3420771 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -137,7 +137,7 @@
>  #define HPD_EVENT_MASK                       0x211c
>  #define HPD_EVENT_DET                        0x2120
>  
> -/* dpyx framer addr */
> +/* dptx framer addr */
>  #define DP_FRAMER_GLOBAL_CONFIG              0x2200
>  #define DP_SW_RESET                  0x2204
>  #define DP_FRAMER_TU                 0x2208
> @@ -431,6 +431,40 @@
>  /* Reference cycles when using lane clock as reference */
>  #define LANE_REF_CYC                         0x8000
>  
> +/* register CM_VID_CTRL */
> +#define LANE_VID_REF_CYC(x)                    (((x) & (BIT(24) - 1)) << 0)
> +#define NMVID_MEAS_TOLERANCE(x)                        (((x) & 0xf) << 24)
> +
> +/* register DP_TX_PHY_CONFIG_REG */
> +#define DP_TX_PHY_TRAINING_ENABLE(x)           ((x) & 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PRBS7          (0 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS1           (1 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS2           (2 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS3           (3 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS4           (4 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PLTPAT         (5 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_D10_2          (6 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_HBR2CPAT       (8 << 1)
> +#define DP_TX_PHY_TRAINING_PATTERN(x)          ((x) << 1)
> +#define DP_TX_PHY_SCRAMBLER_BYPASS(x)          (((x) & 1) << 5)
> +#define DP_TX_PHY_ENCODER_BYPASS(x)            (((x) & 1) << 6)
> +#define DP_TX_PHY_SKEW_BYPASS(x)               (((x) & 1) << 7)
> +#define DP_TX_PHY_DISPARITY_RST(x)             (((x) & 1) << 8)
> +#define DP_TX_PHY_LANE0_SKEW(x)                (((x) & 7) << 9)
> +#define DP_TX_PHY_LANE1_SKEW(x)                (((x) & 7) << 12)
> +#define DP_TX_PHY_LANE2_SKEW(x)                (((x) & 7) << 15)
> +#define DP_TX_PHY_LANE3_SKEW(x)                (((x) & 7) << 18)
> +#define DP_TX_PHY_10BIT_ENABLE(x)              (((x) & 1) << 21)
> +
> +/* register DP_FRAMER_GLOBAL_CONFIG */
> +#define NUM_LANES(x)           ((x) & 3)
> +#define SST_MODE               (0 << 2)
> +#define RG_EN                  (0 << 4)
> +#define GLOBAL_EN              BIT(3)
> +#define NO_VIDEO               BIT(5)
> +#define ENC_RST_DIS            BIT(6)
> +#define WR_VHSYNC_FALL         BIT(7)
> +
>  enum voltage_swing_level {
>       VOLTAGE_LEVEL_0,
>       VOLTAGE_LEVEL_1,
> @@ -476,6 +510,7 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 
> lanes, bool flip);
>  int cdn_dp_event_config(struct cdn_dp_device *dp);
>  u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>  int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val);
>  ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
>                         u8 *data, u16 len);
>  ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> @@ -489,4 +524,5 @@ int cdn_dp_config_video(struct cdn_dp_device *dp);
>  int cdn_dp_audio_stop(struct cdn_dp_device *dp, struct audio_info *audio);
>  int cdn_dp_audio_mute(struct cdn_dp_device *dp, bool enable);
>  int cdn_dp_audio_config(struct cdn_dp_device *dp, struct audio_info *audio);
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp);
>  #endif /* _CDN_DP_REG_H */
> -- 
> 2.7.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to