[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cc969e16-9e1f-823a-c55b-419db1933364@rock-chips.com>
Date: Wed, 23 May 2018 09:14:13 +0800
From: hl <hl@...k-chips.com>
To: Enric Balletbo Serra <eballetbo@...il.com>
Cc: Sean Paul <seanpaul@...omium.org>, David Airlie <airlied@...ux.ie>,
Chris Zhong <zyw@...k-chips.com>,
Kishon Vijay Abraham I <kishon@...com>,
Doug Anderson <dianders@...omium.org>,
Brian Norris <briannorris@...omium.org>,
"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
Heiko Stübner <heiko@...ech.de>,
daniel.vetter@...el.com, jani.nikula@...ux.intel.com,
dri-devel <dri-devel@...ts.freedesktop.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Rob Herring <robh+dt@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v6 5/5] drm/rockchip: support dp training outside dp
firmware
Hi Enric,
On Wednesday, May 23, 2018 01:06 AM, Enric Balletbo Serra wrote:
> Lin,
>
> 2018-05-22 9:41 GMT+02:00 Enric Balletbo Serra <eballetbo@...il.com>:
>> Hi Lin
>>
>> 2018-05-22 3:08 GMT+02:00 hl <hl@...k-chips.com>:
>>> Hi Enric,
>>>
>>>
>>>
>>>
>>> On Monday, May 21, 2018 11:22 PM, Enric Balletbo Serra wrote:
>>>> Hi Lin,
>>>>
>>>> 2018-05-21 11:37 GMT+02:00 Lin Huang <hl@...k-chips.com>:
>>>>> 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 get phy config values from dts and use 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 <zyw@...k-chips.com>
>>>>> Signed-off-by: Lin Huang <hl@...k-chips.com>
>>>>> Reviewed-by: Sean Paul <seanpaul@...omium.org>
>>>>> ---
>>>>> Changes in v2:
>>>>> - update patch following Enric suggest
>>>>> Changes in v3:
>>>>> - use variable fw_training instead sw_training_success
>>>>> - base on DP SPCE, if training fail use lower link rate to retry training
>>>>> Changes in v4:
>>>>> - improve cdn_dp_get_lower_link_rate() and cdn_dp_software_train_link()
>>>>> follow Sean suggest
>>>>> Changes in v5:
>>>>> - fix some whitespcae issue
>>>>> Changes in v6:
>>>>> - None
>>>>>
>>>>> 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 | 420
>>>>> ++++++++++++++++++++++++
>>>>> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 31 +-
>>>>> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
>>>>> 6 files changed, 505 insertions(+), 13 deletions(-)
>>>>> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rockchip/Makefile
>>>>> b/drivers/gpu/drm/rockchip/Makefile
>>>>> index a314e21..b932f62 100644
>>>>> --- a/drivers/gpu/drm/rockchip/Makefile
>>>>> +++ b/drivers/gpu/drm/rockchip/Makefile
>>>>> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
>>>>> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>>>>>
>>>>> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>>>>> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
>>>>> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
>>>>> + cdn-dp-link-training.o
>>>>> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>>>>> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>>>>> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>>> index cce64c1..d9d0d4d 100644
>>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>>> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct
>>>>> drm_encoder *encoder)
>>>>> goto out;
>>>>> }
>>>>> }
>>>>> -
>>>>> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>>> - if (ret) {
>>>>> - DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
>>>>> - goto out;
>>>>> + if (dp->use_fw_training == true) {
>>>> You don't need to compare to true. Simply do:
>>>>
>>>> if (dp->use_fw_training)
>>>>
>>>>> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>>> + if (ret) {
>>>>> + DRM_DEV_ERROR(dp->dev,
>>>>> + "Failed to idle video %d\n", ret);
>>>>> + goto out;
>>>>> + }
>>>>> }
>>>>>
>>>>> ret = cdn_dp_config_video(dp);
>>>>> @@ -642,11 +644,15 @@ static void cdn_dp_encoder_enable(struct
>>>>> drm_encoder *encoder)
>>>>> goto out;
>>>>> }
>>>>>
>>>>> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>>> - if (ret) {
>>>>> - DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n",
>>>>> ret);
>>>>> - goto out;
>>>>> + if (dp->use_fw_training == true) {
>>>> if (dp->use_fw_training)
>>>>
>>>>> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>>> + if (ret) {
>>>>> + DRM_DEV_ERROR(dp->dev,
>>>>> + "Failed to valid video %d\n", ret);
>>>>> + goto out;
>>>>> + }
>>>>> }
>>>>> +
>>>>> out:
>>>>> mutex_unlock(&dp->lock);
>>>>> }
>>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>>> index 46159b2..77a9793 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 use_fw_training;
>>>>>
>>>>> 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;
>>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>> b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>> new file mode 100644
>>>>> index 0000000..73c3290
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>> @@ -0,0 +1,420 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
>>>>> + * Author: Chris Zhong <zyw@...k-chips.com>
>>>>> + */
> Oh, I just noticed that this is not the preferred format for .c files,
> see the discussion here
>
> https://lkml.org/lkml/2017/11/25/133
So what is the standard, the document:
https://www.kernel.org/doc/html/latest/process/license-rules.html
or Linus's word, do we have official about it?
>
>>>>> +
>>>>> +#include <linux/device.h>
>>>>> +#include <linux/delay.h>
>>>>> +#include <linux/phy/phy.h>
>>>>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>>>> +
>>>>> +#include "cdn-dp-core.h"
>>>>> +#include "cdn-dp-reg.h"
>>>>> +
>>>>> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + struct cdn_dp_port *port = dp->port[dp->active_port];
>>>>> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>>>>> +
>>>>> + int rate = drm_dp_bw_code_to_link_rate(dp->link.rate);
>>>>> + u8 swing = (dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
>>>>> + DP_TRAIN_VOLTAGE_SWING_SHIFT;
>>>>> + u8 pre_emphasis = (dp->train_set[0] & DP_TRAIN_PRE_EMPHASIS_MASK)
>>>>> + >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>>>>> +
>>>>> + tcphy->typec_phy_config(port->phy, rate, dp->link.num_lanes,
>>>>> + swing, pre_emphasis);
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_set_pattern(struct cdn_dp_device *dp, uint8_t
>>>>> dp_train_pat)
>>>>> +{
>>>>> + u32 phy_config, global_config;
>>>>> + int ret;
>>>>> + uint8_t pattern = dp_train_pat & DP_TRAINING_PATTERN_MASK;
>>>>> +
>>>>> + global_config = NUM_LANES(dp->link.num_lanes - 1) | SST_MODE |
>>>>> + GLOBAL_EN | RG_EN | ENC_RST_DIS | WR_VHSYNC_FALL;
>>>>> +
>>>>> + phy_config = DP_TX_PHY_ENCODER_BYPASS(0) |
>>>>> + DP_TX_PHY_SKEW_BYPASS(0) |
>>>>> + DP_TX_PHY_DISPARITY_RST(0) |
>>>>> + DP_TX_PHY_LANE0_SKEW(0) |
>>>>> + DP_TX_PHY_LANE1_SKEW(1) |
>>>>> + DP_TX_PHY_LANE2_SKEW(2) |
>>>>> + DP_TX_PHY_LANE3_SKEW(3) |
>>>>> + DP_TX_PHY_10BIT_ENABLE(0);
>>>>> +
>>>>> + if (pattern != DP_TRAINING_PATTERN_DISABLE) {
>>>>> + global_config |= NO_VIDEO;
>>>>> + phy_config |= DP_TX_PHY_TRAINING_ENABLE(1) |
>>>>> + DP_TX_PHY_SCRAMBLER_BYPASS(1) |
>>>>> + DP_TX_PHY_TRAINING_PATTERN(pattern);
>>>>> + }
>>>>> +
>>>>> + ret = cdn_dp_reg_write(dp, DP_FRAMER_GLOBAL_CONFIG,
>>>>> global_config);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("fail to set DP_FRAMER_GLOBAL_CONFIG, error:
>>>>> %d\n",
>>>>> + ret);
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + ret = cdn_dp_reg_write(dp, DP_TX_PHY_CONFIG_REG, phy_config);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("fail to set DP_TX_PHY_CONFIG_REG, error:
>>>>> %d\n",
>>>>> + ret);
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + ret = cdn_dp_reg_write(dp, DPTX_LANE_EN, BIT(dp->link.num_lanes)
>>>>> - 1);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("fail to set DPTX_LANE_EN, error: %d\n", ret);
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>>> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 1);
>>>>> + else
>>>>> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 0);
>>>>> + if (ret)
>>>>> + DRM_ERROR("failed to set DPTX_ENHNCD, error: %x\n", ret);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static u8 cdn_dp_pre_emphasis_max(u8 voltage_swing)
>>>>> +{
>>>>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>>> + default:
>>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void cdn_dp_get_adjust_train(struct cdn_dp_device *dp,
>>>>> + uint8_t
>>>>> link_status[DP_LINK_STATUS_SIZE])
>>>>> +{
>>>>> + int i;
>>>>> + uint8_t v = 0, p = 0;
>>>>> + uint8_t preemph_max;
>>>>> +
>>>>> + for (i = 0; i < dp->link.num_lanes; i++) {
>>>>> + v = max(v, drm_dp_get_adjust_request_voltage(link_status,
>>>>> i));
>>>>> + p = max(p,
>>>>> drm_dp_get_adjust_request_pre_emphasis(link_status,
>>>>> + i));
>>>>> + }
>>>>> +
>>>>> + if (v >= VOLTAGE_LEVEL_2)
>>>>> + v = VOLTAGE_LEVEL_2 | DP_TRAIN_MAX_SWING_REACHED;
>>>>> +
>>>>> + preemph_max = cdn_dp_pre_emphasis_max(v);
>>>>> + if (p >= preemph_max)
>>>>> + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>>>>> +
>>>>> + for (i = 0; i < dp->link.num_lanes; i++)
>>>>> + dp->train_set[i] = v | p;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Pick training pattern for channel equalization. Training Pattern 3
>>>>> for HBR2
>>>>> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
>>>>> + */
>>>>> +static u32 cdn_dp_select_chaneq_pattern(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + u32 training_pattern = DP_TRAINING_PATTERN_2;
>>>>> +
>>>>> + /*
>>>>> + * cdn dp support HBR2 also support TPS3. TPS3 support is also
>>>>> mandatory
>>>>> + * for downstream devices that support HBR2. However, not all
>>>>> sinks
>>>>> + * follow the spec.
>>>>> + */
>>>>> + if (drm_dp_tps3_supported(dp->dpcd))
>>>>> + training_pattern = DP_TRAINING_PATTERN_3;
>>>>> + else
>>>>> + DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3
>>>>> support\n");
>>>>> +
>>>>> + return training_pattern;
>>>>> +}
>>>>> +
>>>>> +
>>>>> +static bool cdn_dp_link_max_vswing_reached(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + int lane;
>>>>> +
>>>>> + for (lane = 0; lane < dp->link.num_lanes; lane++)
>>>>> + if ((dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) ==
>>>>> 0)
>>>>> + return false;
>>>>> +
>>>>> + return true;
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_update_link_train(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + cdn_dp_set_signal_levels(dp);
>>>>> +
>>>>> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
>>>>> + dp->train_set, dp->link.num_lanes);
>>>>> + if (ret != dp->link.num_lanes)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_set_link_train(struct cdn_dp_device *dp,
>>>>> + uint8_t dp_train_pat)
>>>>> +{
>>>>> + uint8_t buf[sizeof(dp->train_set) + 1];
>>>>> + int ret, len;
>>>>> +
>>>>> + buf[0] = dp_train_pat;
>>>>> + if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) ==
>>>>> + DP_TRAINING_PATTERN_DISABLE) {
>>>>> + /* don't write DP_TRAINING_LANEx_SET on disable */
>>>>> + len = 1;
>>>>> + } else {
>>>>> + /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET
>>>>> */
>>>>> + memcpy(buf + 1, dp->train_set, dp->link.num_lanes);
>>>>> + len = dp->link.num_lanes + 1;
>>>>> + }
>>>>> +
>>>>> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET,
>>>>> + buf, len);
>>>>> + if (ret != len)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_reset_link_train(struct cdn_dp_device *dp,
>>>>> + uint8_t dp_train_pat)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + memset(dp->train_set, 0, sizeof(dp->train_set));
>>>>> +
>>>>> + cdn_dp_set_signal_levels(dp);
>>>>> +
>>>>> + ret = cdn_dp_set_pattern(dp, dp_train_pat);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + return cdn_dp_set_link_train(dp, dp_train_pat);
>>>>> +}
>>>>> +
>>>>> +/* Enable corresponding port and start training pattern 1 */
>>>>> +static int cdn_dp_link_training_clock_recovery(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + u8 voltage;
>>>>> + u8 link_status[DP_LINK_STATUS_SIZE];
>>>>> + u32 voltage_tries, max_vswing_tries;
>>>>> + int ret;
>>>>> +
>>>>> + /* clock recovery */
>>>>> + ret = cdn_dp_reset_link_train(dp, DP_TRAINING_PATTERN_1 |
>>>>> + DP_LINK_SCRAMBLING_DISABLE);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("failed to start link train\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + voltage_tries = 1;
>>>>> + max_vswing_tries = 0;
>>>>> + for (;;) {
>>>>> + drm_dp_link_train_clock_recovery_delay(dp->dpcd);
>>>>> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>>> !=
>>>>> + DP_LINK_STATUS_SIZE) {
>>>>> + DRM_ERROR("failed to get link status\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if (drm_dp_clock_recovery_ok(link_status,
>>>>> dp->link.num_lanes)) {
>>>>> + DRM_DEBUG_KMS("clock recovery OK\n");
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + if (voltage_tries >= 5) {
>>>>> + DRM_DEBUG_KMS("Same voltage tried 5 times\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if (max_vswing_tries >= 1) {
>>>>> + DRM_DEBUG_KMS("Max Voltage Swing reached\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + voltage = dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
>>>>> +
>>>>> + /* Update training set as requested by target */
>>>>> + cdn_dp_get_adjust_train(dp, link_status);
>>>>> + if (cdn_dp_update_link_train(dp)) {
>>>>> + DRM_ERROR("failed to update link training\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if ((dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
>>>>> + voltage)
>>>>> + ++voltage_tries;
>>>>> + else
>>>>> + voltage_tries = 1;
>>>>> +
>>>>> + if (cdn_dp_link_max_vswing_reached(dp))
>>>>> + ++max_vswing_tries;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_link_training_channel_equalization(struct
>>>>> cdn_dp_device *dp)
>>>>> +{
>>>>> + int tries, ret;
>>>>> + u32 training_pattern;
>>>>> + uint8_t link_status[DP_LINK_STATUS_SIZE];
>>>>> +
>>>>> + training_pattern = cdn_dp_select_chaneq_pattern(dp);
>>>>> + training_pattern |= DP_LINK_SCRAMBLING_DISABLE;
>>>>> +
>>>>> + ret = cdn_dp_set_pattern(dp, training_pattern);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = cdn_dp_set_link_train(dp, training_pattern);
>>>>> + if (ret) {
>>>>> + DRM_ERROR("failed to start channel equalization\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + for (tries = 0; tries < 5; tries++) {
>>>>> + drm_dp_link_train_channel_eq_delay(dp->dpcd);
>>>>> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>>> !=
>>>>> + DP_LINK_STATUS_SIZE) {
>>>>> + DRM_ERROR("failed to get link status\n");
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + /* Make sure clock is still ok */
>>>>> + if (!drm_dp_clock_recovery_ok(link_status,
>>>>> + dp->link.num_lanes)) {
>>>>> + DRM_DEBUG_KMS("Clock recovery check failed\n");
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (drm_dp_channel_eq_ok(link_status,
>>>>> dp->link.num_lanes)) {
>>>>> + DRM_DEBUG_KMS("Channel EQ done\n");
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + /* Update training set as requested by target */
>>>>> + cdn_dp_get_adjust_train(dp, link_status);
>>>>> + if (cdn_dp_update_link_train(dp)) {
>>>>> + DRM_ERROR("failed to update link training\n");
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* Try 5 times, else fail and try at lower BW */
>>>>> + if (tries == 5)
>>>>> + DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_stop_link_train(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + int ret = cdn_dp_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
>>>>> +
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + return cdn_dp_set_link_train(dp, DP_TRAINING_PATTERN_DISABLE);
>>>>> +}
>>>>> +
>>>>> +static int cdn_dp_get_lower_link_rate(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + switch (dp->link.rate) {
>>>>> + case DP_LINK_BW_1_62:
>>>>> + return -EINVAL;
>>>>> + case DP_LINK_BW_2_7:
>>>>> + dp->link.rate = DP_LINK_BW_1_62;
>>>>> + break;
>>>>> + case DP_LINK_BW_5_4:
>>>>> + dp->link.rate = DP_LINK_BW_2_7;
>>>>> + break;
>>>>> + default:
>>>>> + dp->link.rate = DP_LINK_BW_5_4;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
>>>>> +{
>>>>> + int ret, stop_err;
>>>>> + u8 link_config[2];
>>>>> + u32 rate, sink_max, source_max;
>>>>> +
>>>>> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
>>>>> + sizeof(dp->dpcd));
>>>>> + if (ret < 0) {
>>>>> + DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + source_max = dp->lanes;
>>>>> + sink_max = drm_dp_max_lane_count(dp->dpcd);
>>>>> + dp->link.num_lanes = min(source_max, sink_max);
>>>>> +
>>>>> + source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
>>>>> + sink_max = drm_dp_max_link_rate(dp->dpcd);
>>>>> + rate = min(source_max, sink_max);
>>>>> + dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
>>>>> +
>>>>> + link_config[0] = 0;
>>>>> + link_config[1] = 0;
>>>>> + if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
>>>>> + link_config[1] = DP_SET_ANSI_8B10B;
>>>>> + drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
>>>>> +
>>>>> + while (true) {
>>>>> +
>>>>> + /* Write the link configuration data */
>>>>> + link_config[0] = dp->link.rate;
>>>>> + link_config[1] = dp->link.num_lanes;
>>>>> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>>> + link_config[1] |=
>>>>> DP_LANE_COUNT_ENHANCED_FRAME_EN;
>>>>> + drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config,
>>>>> 2);
>>>>> +
>>>>> + ret = cdn_dp_link_training_clock_recovery(dp);
>>>>> + if (ret) {
>>>>> + if (!cdn_dp_get_lower_link_rate(dp))
>>>>> + continue;
>>>>> +
>>>>> + DRM_ERROR("training clock recovery failed: %d\n",
>>>>> ret);
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + ret = cdn_dp_link_training_channel_equalization(dp);
>>>>> + if (ret) {
>>>>> + if (!cdn_dp_get_lower_link_rate(dp))
>>>>> + continue;
>>>>> +
>>>>> + DRM_ERROR("training channel eq failed: %d\n",
>>>>> ret);
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + stop_err = cdn_dp_stop_link_train(dp);
>>>>> + if (stop_err) {
>>>>> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
>>>>> + return stop_err;
>>>>> + }
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>>> b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>>> index 979355d..e1273e6 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];
>>>>>
>>>>> @@ -609,6 +611,31 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>>>>> {
>>>>> int ret;
>>>>>
>>>>> + /*
>>>>> + * 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.
>>>>> + */
>>>>> + 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;
>>>>> + }
>>>> If I understand correctly you are changing current behavior. Before
>>>> this patch, we use always firmware link training, and after this
>>>> patch, we always use software link training. If fails we use the
>>>> firmware link training.
>>>>
>>>> AFAIK my Samsung Chromebook Plus works well with firmware link
>>>> training, so there are any benefits of use software link training
>>>> instead of firmware link training?
>>>>
>>>> Looks to me that we should only do software link training on these
>>>> platforms that need it, so on those that define the phy-config
>>>> property in their DT and use by default firmware link training.
>>> Sean and me have discussed about that, and we all agree to use sw training
>>> instead the
>>> fw training to keep training process consistent, and we do not need add a
>>> varialbe in
>>> struct rockchip_typec_phy(like use_sw_training before) to distinguish sw and
>>> fw training.
>>> If training process implement correctly, sw training not different with fw
>>> training, and as my
>>> test so far, sw training work well on Kevin, ofcourse, we need keep testing
>>> it.
>>>
>> Ok, I can also confirm that software training works well on kevin.
>> Could you mention this change of behavior in the commit message? I
>> think that after these patches the firmware training is unlikely to
>> happen.
>>
>> Thanks,
>> Enric
>>
>>
>>>>> + 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->use_fw_training = false;
>>>>> + return 0;
>>>>> +
>>>>> +do_fw_training:
>>>>> + dp->use_fw_training = true;
>>>>> + DRM_DEV_DEBUG_KMS(dp->dev, "use fw training\n");
>>>>> ret = cdn_dp_training_start(dp);
>>>>> if (ret) {
>>>>> DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n",
>>>>> ret);
>>>>> @@ -623,7 +650,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
>>>>>
>>>>
>>>
>
>
Powered by blists - more mailing lists