From: Vinod Koul <vkoul@kernel.org>
To: Kuogee Hsieh <quic_khsieh@quicinc.com>
Cc: robdclark@gmail.com, sean@poorly.run, swboyd@chromium.org,
agross@kernel.org, bjorn.andersson@linaro.org,
robh+dt@kernel.org, devicetree@vger.kernel.org,
quic_abhinavk@quicinc.com, aravindh@codeaurora.org,
freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, kishon@ti.com,
p.zabel@pengutronix.de, Kuogee Hsieh <khsieh@codeaurora.org>
Subject: Re: [PATCH v6] phy: qcom-qmp: add display port v4 voltage and pre-emphasis swing tables
Date: Thu, 9 Dec 2021 17:30:53 +0530 [thread overview]
Message-ID: <YbHv9Xv04iEHuaMB@matsya> (raw)
In-Reply-To: <1638998533-3729-1-git-send-email-quic_khsieh@quicinc.com>
On 08-12-21, 13:22, Kuogee Hsieh wrote:
> From: Kuogee Hsieh <khsieh@codeaurora.org>
>
> "add support for sm8250-usb3-dp phy" patch added functions to support V4
^^
why this leading quote here?
> phy. But it did not update voltage and pre-emphasis tables accordingly.
> This patch add v4 voltage and pre-emphasis swing tables to complete v4
> phy implementation. Both voltage and pre-emphasis swing level are set
> during link training negotiation between host and sink. There are totally
> four tables added. A voltage swing table for both hbr and hbr1, a voltage
> table for both hbr2 and hbr3, a pre-emphasis table for both hbr and hbr1
> and a pre-emphasis table for both hbr2 and hbr3. In addition, write 0x0a
> to TX_TX_POL_INV is added to complete the sequence of configure dp phy
> base on HPG.
>
> Chnages in v2:
There is a typo :), as Bjorn said please drop this from non drm code, we
dont need changelog in phy patches
> -- revise commit test
> -- add Fixes tag
> -- replaced voltage_swing_cfg with voltage
> -- replaced pre_emphasis_cfg with emphasis
> -- delete drv_lvl_reg and emp_post_reg parameters from qcom_qmp_v4_phy_configure_dp_swing()
> -- delete drv_lvl_reg and emp_post_reg parameters from qcom_qmp_phy_configure_dp_swing()
>
> Changes in V3:
> -- add __qcom_qmp_phy_configure_dp_swing() to commit swing/pre-emphasis level
>
> Changes in V4:
> -- pass 2D array to __qcom_qmp_phy_configure_dp_swing()
>
> Changes in V5:
> -- rebase on msm-next
>
> Changes in V6:
> -- change commit text title
> -- re wording commit text
>
> Fixes: aff188feb5e1 ("phy: qcom-qmp: add support for sm8250-usb3-dp phy")
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp.c | 97 +++++++++++++++++++++++++------------
> 1 file changed, 66 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 456a59d..1f3585d 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -4283,12 +4283,17 @@ static const u8 qmp_dp_v3_voltage_swing_hbr_rbr[4][4] = {
> { 0x1f, 0xff, 0xff, 0xff }
> };
>
> -static int qcom_qmp_phy_configure_dp_swing(struct qmp_phy *qphy,
> - unsigned int drv_lvl_reg, unsigned int emp_post_reg)
> +static int __qcom_qmp_phy_configure_dp_swing(struct qmp_phy *qphy,
> + unsigned int drv_lvl_reg,
> + unsigned int emp_post_reg,
> + const u8 voltage_swing_hbr_rbr[4][4],
> + const u8 pre_emphasis_hbr_rbr[4][4],
> + const u8 voltage_swing_hbr3_hbr2[4][4],
> + const u8 pre_emphasis_hbr3_hbr2[4][4])
Pls align these to opening brances (hint checkpatch with --strict should
give you a warning)
Second I see that we are hardcoding the 4 here and all over the driver.
I guess it would make sense to define it and use it here and
everywhere..?
> {
> const struct phy_configure_opts_dp *dp_opts = &qphy->dp_opts;
> unsigned int v_level = 0, p_level = 0;
> - u8 voltage_swing_cfg, pre_emphasis_cfg;
> + u8 voltage, emphasis;
> int i;
>
> for (i = 0; i < dp_opts->lanes; i++) {
> @@ -4297,26 +4302,25 @@ static int qcom_qmp_phy_configure_dp_swing(struct qmp_phy *qphy,
> }
>
> if (dp_opts->link_rate <= 2700) {
> - voltage_swing_cfg = qmp_dp_v3_voltage_swing_hbr_rbr[v_level][p_level];
> - pre_emphasis_cfg = qmp_dp_v3_pre_emphasis_hbr_rbr[v_level][p_level];
> + voltage = voltage_swing_hbr_rbr[v_level][p_level];
> + emphasis = pre_emphasis_hbr_rbr[v_level][p_level];
> } else {
> - voltage_swing_cfg = qmp_dp_v3_voltage_swing_hbr3_hbr2[v_level][p_level];
> - pre_emphasis_cfg = qmp_dp_v3_pre_emphasis_hbr3_hbr2[v_level][p_level];
> + voltage = voltage_swing_hbr3_hbr2[v_level][p_level];
> + emphasis = pre_emphasis_hbr3_hbr2[v_level][p_level];
> }
>
> /* TODO: Move check to config check */
> - if (voltage_swing_cfg == 0xFF && pre_emphasis_cfg == 0xFF)
> + if (voltage == 0xFF && emphasis == 0xFF)
> return -EINVAL;
>
> /* Enable MUX to use Cursor values from these registers */
> - voltage_swing_cfg |= DP_PHY_TXn_TX_DRV_LVL_MUX_EN;
> - pre_emphasis_cfg |= DP_PHY_TXn_TX_EMP_POST1_LVL_MUX_EN;
> -
> - writel(voltage_swing_cfg, qphy->tx + drv_lvl_reg);
> - writel(pre_emphasis_cfg, qphy->tx + emp_post_reg);
> - writel(voltage_swing_cfg, qphy->tx2 + drv_lvl_reg);
> - writel(pre_emphasis_cfg, qphy->tx2 + emp_post_reg);
> + voltage |= DP_PHY_TXn_TX_DRV_LVL_MUX_EN;
> + emphasis |= DP_PHY_TXn_TX_EMP_POST1_LVL_MUX_EN;
>
> + writel(voltage, qphy->tx + drv_lvl_reg);
> + writel(emphasis, qphy->tx + emp_post_reg);
> + writel(voltage, qphy->tx2 + drv_lvl_reg);
> + writel(emphasis, qphy->tx2 + emp_post_reg);
> return 0;
> }
>
> @@ -4325,9 +4329,13 @@ static void qcom_qmp_v3_phy_configure_dp_tx(struct qmp_phy *qphy)
> const struct phy_configure_opts_dp *dp_opts = &qphy->dp_opts;
> u32 bias_en, drvr_en;
>
> - if (qcom_qmp_phy_configure_dp_swing(qphy,
> - QSERDES_V3_TX_TX_DRV_LVL,
> - QSERDES_V3_TX_TX_EMP_POST1_LVL) < 0)
> + if (__qcom_qmp_phy_configure_dp_swing(qphy,
> + QSERDES_V3_TX_TX_DRV_LVL,
> + QSERDES_V3_TX_TX_EMP_POST1_LVL,
> + qmp_dp_v3_voltage_swing_hbr_rbr,
> + qmp_dp_v3_pre_emphasis_hbr_rbr,
> + qmp_dp_v3_voltage_swing_hbr3_hbr2,
> + qmp_dp_v3_pre_emphasis_hbr3_hbr2) < 0)
> return;
>
> if (dp_opts->lanes == 1) {
> @@ -4465,6 +4473,35 @@ static int qcom_qmp_v3_dp_phy_calibrate(struct qmp_phy *qphy)
> return 0;
> }
>
> +/* The values in these tables are given without MUX_EN (0x20) bit set */
> +static const u8 qmp_dp_v4_pre_emphasis_hbr3_hbr2[4][4] = {
> + { 0x00, 0x0c, 0x15, 0x1b },
> + { 0x02, 0x0e, 0x16, 0xff },
> + { 0x02, 0x11, 0xff, 0xff },
> + { 0x04, 0xff, 0xff, 0xff }
> +};
> +
> +static const u8 qmp_dp_v4_voltage_swing_hbr3_hbr2[4][4] = {
> + { 0x02, 0x12, 0x16, 0x1a },
> + { 0x09, 0x19, 0x1f, 0xff },
> + { 0x10, 0x1f, 0xff, 0xff },
> + { 0x1f, 0xff, 0xff, 0xff }
> +};
> +
> +static const u8 qmp_dp_v4_pre_emphasis_hbr_rbr[4][4] = {
> + { 0x00, 0x0e, 0x15, 0x1b },
> + { 0x00, 0x0e, 0x15, 0xff },
> + { 0x00, 0x0e, 0xff, 0xff },
> + { 0x04, 0xff, 0xff, 0xff }
> +};
> +
> +static const u8 qmp_dp_v4_voltage_swing_hbr_rbr[4][4] = {
> + { 0x08, 0x0f, 0x16, 0x1f },
> + { 0x11, 0x1e, 0x1f, 0xff },
> + { 0x16, 0x1f, 0xff, 0xff },
> + { 0x1f, 0xff, 0xff, 0xff }
> +};
> +
> static void qcom_qmp_v4_phy_dp_aux_init(struct qmp_phy *qphy)
> {
> writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_PSR_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> @@ -4494,16 +4531,13 @@ static void qcom_qmp_v4_phy_dp_aux_init(struct qmp_phy *qphy)
>
> static void qcom_qmp_v4_phy_configure_dp_tx(struct qmp_phy *qphy)
> {
> - /* Program default values before writing proper values */
> - writel(0x27, qphy->tx + QSERDES_V4_TX_TX_DRV_LVL);
> - writel(0x27, qphy->tx2 + QSERDES_V4_TX_TX_DRV_LVL);
> -
> - writel(0x20, qphy->tx + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> - writel(0x20, qphy->tx2 + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> -
> - qcom_qmp_phy_configure_dp_swing(qphy,
> + __qcom_qmp_phy_configure_dp_swing(qphy,
> QSERDES_V4_TX_TX_DRV_LVL,
> - QSERDES_V4_TX_TX_EMP_POST1_LVL);
> + QSERDES_V4_TX_TX_EMP_POST1_LVL,
> + qmp_dp_v4_voltage_swing_hbr_rbr,
> + qmp_dp_v4_pre_emphasis_hbr_rbr,
> + qmp_dp_v4_voltage_swing_hbr3_hbr2,
> + qmp_dp_v4_pre_emphasis_hbr3_hbr2);
> }
>
> static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_phy *qphy)
> @@ -4622,6 +4656,9 @@ static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_phy *qphy)
> writel(drvr1_en, qphy->tx2 + QSERDES_V4_TX_HIGHZ_DRVR_EN);
> writel(bias1_en, qphy->tx2 + QSERDES_V4_TX_TRANSCEIVER_BIAS_EN);
>
> + writel(0x0a, qphy->tx + QSERDES_V4_TX_TX_POL_INV);
> + writel(0x0a, qphy->tx2 + QSERDES_V4_TX_TX_POL_INV);
> +
> writel(0x18, qphy->pcs + QSERDES_DP_PHY_CFG);
> udelay(2000);
> writel(0x19, qphy->pcs + QSERDES_DP_PHY_CFG);
> @@ -4633,11 +4670,9 @@ static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_phy *qphy)
> 10000))
> return -ETIMEDOUT;
>
> - writel(0x0a, qphy->tx + QSERDES_V4_TX_TX_POL_INV);
> - writel(0x0a, qphy->tx2 + QSERDES_V4_TX_TX_POL_INV);
>
> - writel(0x27, qphy->tx + QSERDES_V4_TX_TX_DRV_LVL);
> - writel(0x27, qphy->tx2 + QSERDES_V4_TX_TX_DRV_LVL);
> + writel(0x22, qphy->tx + QSERDES_V4_TX_TX_DRV_LVL);
> + writel(0x22, qphy->tx2 + QSERDES_V4_TX_TX_DRV_LVL);
>
> writel(0x20, qphy->tx + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> writel(0x20, qphy->tx2 + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
~Vinod
prev parent reply other threads:[~2021-12-09 12:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 21:22 [PATCH v6] phy: qcom-qmp: add display port v4 voltage and pre-emphasis swing tables Kuogee Hsieh
2021-12-08 21:47 ` Bjorn Andersson
2021-12-09 12:00 ` Vinod Koul [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YbHv9Xv04iEHuaMB@matsya \
--to=vkoul@kernel.org \
--cc=agross@kernel.org \
--cc=aravindh@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=freedreno@lists.freedesktop.org \
--cc=khsieh@codeaurora.org \
--cc=kishon@ti.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=quic_abhinavk@quicinc.com \
--cc=quic_khsieh@quicinc.com \
--cc=robdclark@gmail.com \
--cc=robh+dt@kernel.org \
--cc=sean@poorly.run \
--cc=swboyd@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.