Re: [Freedreno] [PATCH v6] phy: qcom-qmp: add display port v4 voltage and pre-emphasis swing tables
On 08-12-21, 13:22, Kuogee Hsieh wrote:
> From: Kuogee Hsieh
>
> "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
> Reviewed-by: Stephen Boyd
> ---
> 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 = >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);
> +
Re: [Freedreno] [PATCH v6] phy: qcom-qmp: add display port v4 voltage and pre-emphasis swing tables
On Wed 08 Dec 13:22 PST 2021, Kuogee Hsieh wrote:
> From: Kuogee Hsieh
>
> "add support for sm8250-usb3-dp phy" patch added functions to support V4
> 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:
> -- 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
The changelog still don't belong in the commit message, perhaps Vinod
can drop that as he applies the patch.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
>
> Fixes: aff188feb5e1 ("phy: qcom-qmp: add support for sm8250-usb3-dp phy")
> Signed-off-by: Kuogee Hsieh
> Reviewed-by: Stephen Boyd
> ---
> 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])
> {
> const struct phy_configure_opts_dp *dp_opts = >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 = >dp_opts;
