Until now, all platform that supported both eDP and DP had different
compatibles for each mode. Using different compatibles for basically
the same IP block but for a different configuration is bad way all
around. There is a new compute platform from Qualcomm that supports
both eDP and DP with the same PHY. So instead of following the old
method, we should allow the submode to be configured via set_mode from
the controller driver.
The controller part will follow after we conclude the PHY part first.
Signed-off-by: Abel Vesa <[email protected]>
---
Changes in v5:
- Dropped the unnecessary ternary operator.
- Link to v4: https://lore.kernel.org/r/20240216-x1e80100-phy-edp-compatible-refactor-v4-0-c07fd1a52186@linaro.org
Changes in v4:
- Added todo comment about setting the value of cfg8 based on swing
pre-emph availability, like Konrad suggested
- Fixed the condition in qcom_edp_phy_power_on, reported by Dmitry
- Link to v3: https://lore.kernel.org/r/20240129-x1e80100-phy-edp-compatible-refactor-v3-0-e71f3359c535@linaro.org
Changes in v3:
- Dropped needs_swing_pre_emph_cfg as we store the table instead
- Picking the table based on is_edp instead of overriding.
- Link to v2: https://lore.kernel.org/r/20231222-x1e80100-phy-edp-compatible-refactor-v2-0-ab5786c2359f@linaro.org
Changes in v2:
- Dropped the dedicated xlate function and added set_mode op instead
- Dropped the eDP PHY type and mode addition
- Added the DP PHY submodes (eDP and DP)
- Removed the device match data storing from the container struct
- Link to v1: https://lore.kernel.org/r/20231219-x1e80100-phy-edp-compatible-refactor-v1-0-f9e77752953d@linaro.org
Initial attepmpt was here:
https://lore.kernel.org/all/[email protected]/
Compared to that version, this one uses the phy-cells method and drops
the X1E80100 support. The X1E80100 support will be a separate patchset.
---
Abel Vesa (2):
phy: Add Embedded DisplayPort and DisplayPort submodes
phy: qcom: edp: Add set_mode op for configuring eDP/DP submode
drivers/phy/qualcomm/phy-qcom-edp.c | 76 +++++++++++++++++++++++++++----------
include/linux/phy/phy-dp.h | 3 ++
2 files changed, 59 insertions(+), 20 deletions(-)
---
base-commit: 2d5c7b7eb345249cb34d42cbc2b97b4c57ea944e
change-id: 20231219-x1e80100-phy-edp-compatible-refactor-8733eca7ccda
Best regards,
--
Abel Vesa <[email protected]>
In some cases, a DP PHY needs to be configured to work in eDP mode.
So add submodes for both DP and eDP so they can be used by the
controllers for specifying the mode the PHY should be configured in.
Signed-off-by: Abel Vesa <[email protected]>
---
include/linux/phy/phy-dp.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/phy/phy-dp.h b/include/linux/phy/phy-dp.h
index 18cad23642cd..9cce5766bc0b 100644
--- a/include/linux/phy/phy-dp.h
+++ b/include/linux/phy/phy-dp.h
@@ -8,6 +8,9 @@
#include <linux/types.h>
+#define PHY_SUBMODE_DP 0
+#define PHY_SUBMODE_EDP 1
+
/**
* struct phy_configure_opts_dp - DisplayPort PHY configuration set
*
--
2.34.1
Future platforms should not use different compatibles to differentiate
between eDP and DP mode. Instead, they should use a single compatible as
the IP block is the same. It will be the job of the controller to set the
submode of the PHY accordingly.
The existing platforms will remain with separate compatibles for each
mode.
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-edp.c | 76 +++++++++++++++++++++++++++----------
1 file changed, 56 insertions(+), 20 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index 9818d994c68b..621d0453bf76 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -14,6 +14,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/phy/phy.h>
+#include <linux/phy/phy-dp.h>
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
@@ -69,19 +70,21 @@
#define TXn_TRAN_DRVR_EMP_EN 0x0078
-struct qcom_edp_cfg {
- bool is_dp;
-
- /* DP PHY swing and pre_emphasis tables */
+struct qcom_edp_swing_pre_emph_cfg {
const u8 (*swing_hbr_rbr)[4][4];
const u8 (*swing_hbr3_hbr2)[4][4];
const u8 (*pre_emphasis_hbr_rbr)[4][4];
const u8 (*pre_emphasis_hbr3_hbr2)[4][4];
};
+struct qcom_edp_phy_cfg {
+ bool is_edp;
+ const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
+};
+
struct qcom_edp {
struct device *dev;
- const struct qcom_edp_cfg *cfg;
+ const struct qcom_edp_phy_cfg *cfg;
struct phy *phy;
@@ -97,6 +100,8 @@ struct qcom_edp {
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+
+ bool is_edp;
};
static const u8 dp_swing_hbr_rbr[4][4] = {
@@ -127,8 +132,7 @@ static const u8 dp_pre_emp_hbr2_hbr3[4][4] = {
{ 0x04, 0xff, 0xff, 0xff }
};
-static const struct qcom_edp_cfg dp_phy_cfg = {
- .is_dp = true,
+static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg = {
.swing_hbr_rbr = &dp_swing_hbr_rbr,
.swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3,
.pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr,
@@ -163,18 +167,28 @@ static const u8 edp_pre_emp_hbr2_hbr3[4][4] = {
{ 0x00, 0xff, 0xff, 0xff }
};
-static const struct qcom_edp_cfg edp_phy_cfg = {
- .is_dp = false,
+static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
.swing_hbr_rbr = &edp_swing_hbr_rbr,
.swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3,
.pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr,
.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
};
+static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
+};
+
+static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
+ .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
+};
+
+static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
+ .is_edp = true,
+ .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
+};
+
static int qcom_edp_phy_init(struct phy *phy)
{
struct qcom_edp *edp = phy_get_drvdata(phy);
- const struct qcom_edp_cfg *cfg = edp->cfg;
int ret;
u8 cfg8;
@@ -201,7 +215,12 @@ static int qcom_edp_phy_init(struct phy *phy)
DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
edp->edp + DP_PHY_PD_CTL);
- if (cfg && cfg->is_dp)
+ /*
+ * TODO: Re-work the conditions around setting the cfg8 value
+ * when more information becomes available about why this is
+ * even needed.
+ */
+ if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
cfg8 = 0xb7;
else
cfg8 = 0x37;
@@ -235,7 +254,7 @@ static int qcom_edp_phy_init(struct phy *phy)
static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configure_opts_dp *dp_opts)
{
- const struct qcom_edp_cfg *cfg = edp->cfg;
+ const struct qcom_edp_swing_pre_emph_cfg *cfg = edp->cfg->swing_pre_emph_cfg;
unsigned int v_level = 0;
unsigned int p_level = 0;
u8 ldo_config;
@@ -246,6 +265,9 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
if (!cfg)
return 0;
+ if (edp->is_edp)
+ cfg = &edp_phy_swing_pre_emph_cfg;
+
for (i = 0; i < dp_opts->lanes; i++) {
v_level = max(v_level, dp_opts->voltage[i]);
p_level = max(p_level, dp_opts->pre[i]);
@@ -262,7 +284,7 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
if (swing == 0xff || emph == 0xff)
return -EINVAL;
- ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+ ldo_config = edp->is_edp ? 0x0 : 0x1;
writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
@@ -448,10 +470,9 @@ static int qcom_edp_set_vco_div(const struct qcom_edp *edp, unsigned long *pixel
static int qcom_edp_phy_power_on(struct phy *phy)
{
const struct qcom_edp *edp = phy_get_drvdata(phy);
- const struct qcom_edp_cfg *cfg = edp->cfg;
u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
unsigned long pixel_freq;
- u8 ldo_config;
+ u8 ldo_config = 0x0;
int timeout;
int ret;
u32 val;
@@ -469,7 +490,8 @@ static int qcom_edp_phy_power_on(struct phy *phy)
return timeout;
- ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+ if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
+ ldo_config = 0x1;
writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
@@ -590,6 +612,18 @@ static int qcom_edp_phy_power_off(struct phy *phy)
return 0;
}
+static int qcom_edp_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+ struct qcom_edp *edp = phy_get_drvdata(phy);
+
+ if (mode != PHY_MODE_DP)
+ return -EINVAL;
+
+ edp->is_edp = submode == PHY_SUBMODE_EDP;
+
+ return 0;
+}
+
static int qcom_edp_phy_exit(struct phy *phy)
{
struct qcom_edp *edp = phy_get_drvdata(phy);
@@ -605,6 +639,7 @@ static const struct phy_ops qcom_edp_ops = {
.configure = qcom_edp_phy_configure,
.power_on = qcom_edp_phy_power_on,
.power_off = qcom_edp_phy_power_off,
+ .set_mode = qcom_edp_phy_set_mode,
.exit = qcom_edp_phy_exit,
.owner = THIS_MODULE,
};
@@ -782,6 +817,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
edp->dev = dev;
edp->cfg = of_device_get_match_data(&pdev->dev);
+ edp->is_edp = edp->cfg->is_edp;
edp->edp = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(edp->edp))
@@ -840,10 +876,10 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
}
static const struct of_device_id qcom_edp_phy_match_table[] = {
- { .compatible = "qcom,sc7280-edp-phy" },
- { .compatible = "qcom,sc8180x-edp-phy" },
- { .compatible = "qcom,sc8280xp-dp-phy", .data = &dp_phy_cfg },
- { .compatible = "qcom,sc8280xp-edp-phy", .data = &edp_phy_cfg },
+ { .compatible = "qcom,sc7280-edp-phy", .data = &sc7280_dp_phy_cfg, },
+ { .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
+ { .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
+ { .compatible = "qcom,sc8280xp-edp-phy", .data = &sc8280xp_edp_phy_cfg, },
{ }
};
MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
--
2.34.1
On Sun, 24 Mar 2024 at 20:50, Abel Vesa <[email protected]> wrote:
>
> In some cases, a DP PHY needs to be configured to work in eDP mode.
> So add submodes for both DP and eDP so they can be used by the
> controllers for specifying the mode the PHY should be configured in.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> include/linux/phy/phy-dp.h | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Dmitry Baryshkov <[email protected]>
Vinod, we'd need an immutable branch or a tag for this patch, so that
we can merge DP changes during this cycle
--
With best wishes
Dmitry
On 24-03-24, 23:26, Dmitry Baryshkov wrote:
> On Sun, 24 Mar 2024 at 20:50, Abel Vesa <[email protected]> wrote:
> >
> > In some cases, a DP PHY needs to be configured to work in eDP mode.
> > So add submodes for both DP and eDP so they can be used by the
> > controllers for specifying the mode the PHY should be configured in.
> >
> > Signed-off-by: Abel Vesa <[email protected]>
> > ---
> > include/linux/phy/phy-dp.h | 3 +++
> > 1 file changed, 3 insertions(+)
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
>
> Vinod, we'd need an immutable branch or a tag for this patch, so that
> we can merge DP changes during this cycle
The following changes since commit 4cece764965020c22cff7665b18a012006359095:
Linux 6.9-rc1 (2024-03-24 14:10:05 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tags/phy_dp_modes_6.10
for you to fetch changes up to 6078b8ce070cad3f9a8ecfce65d0f6d595494f02:
phy: qcom: edp: Add set_mode op for configuring eDP/DP submode (2024-03-29 00:30:16 +0530)
----------------------------------------------------------------
phy_dp_modes_6.10
This contains the dp submode definition and associated qcom driver change
----------------------------------------------------------------
Abel Vesa (2):
phy: Add Embedded DisplayPort and DisplayPort submodes
phy: qcom: edp: Add set_mode op for configuring eDP/DP submode
drivers/phy/qualcomm/phy-qcom-edp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------
include/linux/phy/phy-dp.h | 3 +++
2 files changed, 59 insertions(+), 20 deletions(-)
--
~Vinod
On Sun, 24 Mar 2024 20:50:16 +0200, Abel Vesa wrote:
> Until now, all platform that supported both eDP and DP had different
> compatibles for each mode. Using different compatibles for basically
> the same IP block but for a different configuration is bad way all
> around. There is a new compute platform from Qualcomm that supports
> both eDP and DP with the same PHY. So instead of following the old
> method, we should allow the submode to be configured via set_mode from
> the controller driver.
>
> [...]
Applied, thanks!
[1/2] phy: Add Embedded DisplayPort and DisplayPort submodes
commit: 368d67dab4cc4a3ffd39fbd062b2f5796cdbb37b
[2/2] phy: qcom: edp: Add set_mode op for configuring eDP/DP submode
commit: 6078b8ce070cad3f9a8ecfce65d0f6d595494f02
Best regards,
--
~Vinod