1) add regulator_set_load() to eDP phy 2) add regulator_set_load() to DP phy 3) remove vdda related function out of eDP/DP controller
Kuogee Hsieh (3): phy: qcom-edp: add regulator_set_load to edp phy phy: qcom-qmp: add regulator_set_load to dp phy drm/msm/dp: delete vdda regulator related functions from eDP/DP controller
drivers/gpu/drm/msm/dp/dp_parser.c | 14 ------ drivers/gpu/drm/msm/dp/dp_parser.h | 6 --- drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------------ drivers/phy/qualcomm/phy-qcom-edp.c | 12 +++++ drivers/phy/qualcomm/phy-qcom-qmp.c | 25 +++++++++- 5 files changed, 38 insertions(+), 114 deletions(-)
This patch add regulator_set_load() before enable regulator at eDP phy driver.
Signed-off-by: Kuogee Hsieh quic_khsieh@quicinc.com --- drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c index cacd32f..7e357078 100644 --- a/drivers/phy/qualcomm/phy-qcom-edp.c +++ b/drivers/phy/qualcomm/phy-qcom-edp.c @@ -639,6 +639,18 @@ static int qcom_edp_phy_probe(struct platform_device *pdev) if (ret) return ret;
+ ret = regulator_set_load(edp->supplies[0].consumer, 21800); /* 1.2 V vdda-phy */ + if (ret) { + dev_err(dev, "failed to set load at %s\n", edp->supplies[0].supply); + return ret; + } + + ret = regulator_set_load(edp->supplies[1].consumer, 36000); /* 0.9 V vdda-pll */ + if (ret) { + dev_err(dev, "failed to set load at %s\n", edp->supplies[1].supply); + return ret; + } + ret = qcom_edp_clks_register(edp, pdev->dev.of_node); if (ret) return ret;
Hi,
On Fri, May 20, 2022 at 2:28 PM Kuogee Hsieh quic_khsieh@quicinc.com wrote:
This patch add regulator_set_load() before enable regulator at eDP phy driver.
Signed-off-by: Kuogee Hsieh quic_khsieh@quicinc.com
drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Douglas Anderson dianders@chromium.org
This patch add regulator_set_load() before enable regulator at DP phy driver.
Signed-off-by: Kuogee Hsieh quic_khsieh@quicinc.com Reviewed-by: Stephen Boyd swboyd@chromium.org --- drivers/phy/qualcomm/phy-qcom-qmp.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index b144ae1..a93e153 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -3130,6 +3130,7 @@ struct qmp_phy_cfg { int num_resets; /* regulators to be requested */ const char * const *vreg_list; + const unsigned int *vreg_enable_load; int num_vregs;
/* array of registers with different offsets */ @@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = { "vdda-phy", "vdda-pll", };
+static const unsigned int qmp_phy_vreg_enable_load[] = { + 21800, 36000 +}; + static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = { .type = PHY_TYPE_USB3, .nlanes = 1, @@ -3711,6 +3716,7 @@ static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { .reset_list = sc7180_usb3phy_reset_l, .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v3_usb3phy_regs_layout,
@@ -3749,6 +3755,7 @@ static const struct qmp_phy_cfg sc7180_dpphy_cfg = { .reset_list = sc7180_usb3phy_reset_l, .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v3_usb3phy_regs_layout,
@@ -3940,6 +3947,7 @@ static const struct qmp_phy_cfg sm8150_usb3phy_cfg = { .reset_list = msm8996_usb3phy_reset_l, .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v4_usb3phy_regs_layout,
@@ -4009,6 +4017,7 @@ static const struct qmp_phy_cfg sc8180x_dpphy_cfg = { .reset_list = sc7180_usb3phy_reset_l, .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v3_usb3phy_regs_layout,
@@ -4072,6 +4081,7 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = { .reset_list = msm8996_usb3phy_reset_l, .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v4_usb3phy_regs_layout,
@@ -4139,6 +4149,7 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = { .reset_list = msm8996_usb3phy_reset_l, .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l, + .vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l), .regs = qmp_v4_usb3phy_regs_layout,
@@ -6107,7 +6118,7 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev) const struct qmp_phy_cfg *usb_cfg = NULL; const struct qmp_phy_cfg *dp_cfg = NULL; int num, id, expected_phys; - int ret; + int ret, i;
qmp = devm_kzalloc(dev, sizeof(*qmp), GFP_KERNEL); if (!qmp) @@ -6175,6 +6186,18 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev) return ret; }
+ if (cfg->vreg_enable_load) { + for (i = 0; i < cfg->num_vregs; i++) { + ret = regulator_set_load(qmp->vregs[i].consumer, + cfg->vreg_enable_load[i]); + if (ret) { + dev_err(dev, "failed to set load at %s\n", + qmp->vregs[i].supply); + return ret; + } + } + } + num = of_get_available_child_count(dev->of_node); /* do we have a rogue child node ? */ if (num > expected_phys)
Hi,
On Fri, May 20, 2022 at 2:27 PM Kuogee Hsieh quic_khsieh@quicinc.com wrote:
This patch add regulator_set_load() before enable regulator at DP phy driver.
Signed-off-by: Kuogee Hsieh quic_khsieh@quicinc.com Reviewed-by: Stephen Boyd swboyd@chromium.org
drivers/phy/qualcomm/phy-qcom-qmp.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index b144ae1..a93e153 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -3130,6 +3130,7 @@ struct qmp_phy_cfg { int num_resets; /* regulators to be requested */ const char * const *vreg_list;
const unsigned int *vreg_enable_load; int num_vregs; /* array of registers with different offsets */@@ -3346,6 +3347,10 @@ static const char * const qmp_phy_vreg_l[] = { "vdda-phy", "vdda-pll", };
+static const unsigned int qmp_phy_vreg_enable_load[] = {
21800, 36000+};
I'm a little confused. Why make a new parallel structure? Don't you want to set a load for everyone who's using "qmp_phy_vreg_l"? It seems like it would be better to do something like this:
struct qmp_regulator_data { const char *name; unsigned int load; };
struct qmp_regulator_data qmp_phy_vreg_l[] = { { .name = "vdda-phy", .load = 21800 }, { .name = "vdda-pll", .load = 36000 }, };
Right now some random smattering of devices are setting the load but not others...
static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = { .type = PHY_TYPE_USB3, .nlanes = 1, @@ -3711,6 +3716,7 @@ static const struct qmp_phy_cfg sc7180_usb3phy_cfg = { .reset_list = sc7180_usb3phy_reset_l, .num_resets = ARRAY_SIZE(sc7180_usb3phy_reset_l), .vreg_list = qmp_phy_vreg_l,
.vreg_enable_load = qmp_phy_vreg_enable_load, .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
One downside of the parallel structures is that there's nothing enforcing that ARRAY_SIZE(qmp_phy_vreg_l) == ARRAY_SIZE(qmp_phy_vreg_enable_load), though the code below relies on it.
@@ -6175,6 +6186,18 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev) return ret; }
if (cfg->vreg_enable_load) {for (i = 0; i < cfg->num_vregs; i++) {ret = regulator_set_load(qmp->vregs[i].consumer,cfg->vreg_enable_load[i]);if (ret) {dev_err(dev, "failed to set load at %s\n",qmp->vregs[i].supply);
nit: indentation of the 2nd line seems a bit off?
return ret;}}}
Feels like the above snippet belongs in qcom_qmp_phy_vreg_init() ?
Vdda regulators are related to both eDP and DP phy so that it should be managed at eDP and DP phy driver instead of controller. This patch removes vdda regulators related functions out of eDP/DP controller.
Signed-off-by: Kuogee Hsieh quic_khsieh@quicinc.com Reviewed-by: Stephen Boyd swboyd@chromium.org Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/dp/dp_parser.c | 14 ------ drivers/gpu/drm/msm/dp/dp_parser.h | 6 --- drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------------- 3 files changed, 2 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c index 8f9fed9..4ef2130 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.c +++ b/drivers/gpu/drm/msm/dp/dp_parser.c @@ -22,14 +22,6 @@ #define DP_DEFAULT_P0_OFFSET 0x1000 #define DP_DEFAULT_P0_SIZE 0x0400
-static const struct dp_regulator_cfg sdm845_dp_reg_cfg = { - .num = 2, - .regs = { - {"vdda-1p2", 21800, 4 }, /* 1.2 V */ - {"vdda-0p9", 36000, 32 }, /* 0.9 V */ - }, -}; - static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t *len) { struct resource *res; @@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser) if (rc) return rc;
- /* Map the corresponding regulator information according to - * version. Currently, since we only have one supported platform, - * mapping the regulator directly. - */ - parser->regulator_cfg = &sdm845_dp_reg_cfg; - return 0; }
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h index 3a4d797..b56b4d7 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -101,11 +101,6 @@ struct dp_reg_entry { int disable_load; };
-struct dp_regulator_cfg { - int num; - struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX]; -}; - /** * struct dp_parser - DP parser's data exposed to clients * @@ -121,7 +116,6 @@ struct dp_parser { struct dp_pinctrl pinctrl; struct dp_io io; struct dp_display_data disp_data; - const struct dp_regulator_cfg *regulator_cfg; u32 max_dp_lanes; struct drm_bridge *next_bridge;
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index d9e0117..b52ac1d 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -20,82 +20,10 @@ struct dp_power_private { struct clk *link_clk_src; struct clk *pixel_provider; struct clk *link_provider; - struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
struct dp_power dp_power; };
-static void dp_power_regulator_disable(struct dp_power_private *power) -{ - struct regulator_bulk_data *s = power->supplies; - const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs; - int num = power->parser->regulator_cfg->num; - int i; - - DBG(""); - for (i = num - 1; i >= 0; i--) - if (regs[i].disable_load >= 0) - regulator_set_load(s[i].consumer, - regs[i].disable_load); - - regulator_bulk_disable(num, s); -} - -static int dp_power_regulator_enable(struct dp_power_private *power) -{ - struct regulator_bulk_data *s = power->supplies; - const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs; - int num = power->parser->regulator_cfg->num; - int ret, i; - - DBG(""); - for (i = 0; i < num; i++) { - if (regs[i].enable_load >= 0) { - ret = regulator_set_load(s[i].consumer, - regs[i].enable_load); - if (ret < 0) { - pr_err("regulator %d set op mode failed, %d\n", - i, ret); - goto fail; - } - } - } - - ret = regulator_bulk_enable(num, s); - if (ret < 0) { - pr_err("regulator enable failed, %d\n", ret); - goto fail; - } - - return 0; - -fail: - for (i--; i >= 0; i--) - regulator_set_load(s[i].consumer, regs[i].disable_load); - return ret; -} - -static int dp_power_regulator_init(struct dp_power_private *power) -{ - struct regulator_bulk_data *s = power->supplies; - const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs; - struct platform_device *pdev = power->pdev; - int num = power->parser->regulator_cfg->num; - int i, ret; - - for (i = 0; i < num; i++) - s[i].supply = regs[i].name; - - ret = devm_regulator_bulk_get(&pdev->dev, num, s); - if (ret < 0) { - pr_err("%s: failed to init regulator, ret=%d\n", - __func__, ret); - return ret; - } - - return 0; -} - static int dp_power_clk_init(struct dp_power_private *power) { int rc = 0; @@ -318,21 +246,10 @@ int dp_power_client_init(struct dp_power *dp_power)
pm_runtime_enable(&power->pdev->dev);
- rc = dp_power_regulator_init(power); - if (rc) { - DRM_ERROR("failed to init regulators %d\n", rc); - goto error; - } - rc = dp_power_clk_init(power); - if (rc) { + if (rc) DRM_ERROR("failed to init clocks %d\n", rc); - goto error; - } - return 0;
-error: - pm_runtime_disable(&power->pdev->dev); return rc; }
@@ -365,22 +282,15 @@ int dp_power_init(struct dp_power *dp_power, bool flip) power = container_of(dp_power, struct dp_power_private, dp_power);
pm_runtime_get_sync(&power->pdev->dev); - rc = dp_power_regulator_enable(power); - if (rc) { - DRM_ERROR("failed to enable regulators, %d\n", rc); - goto exit; - }
rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true); if (rc) { DRM_ERROR("failed to enable DP core clocks, %d\n", rc); - goto err_clk; + goto exit; }
return 0;
-err_clk: - dp_power_regulator_disable(power); exit: pm_runtime_put_sync(&power->pdev->dev); return rc; @@ -393,7 +303,6 @@ int dp_power_deinit(struct dp_power *dp_power) power = container_of(dp_power, struct dp_power_private, dp_power);
dp_power_clk_enable(dp_power, DP_CORE_PM, false); - dp_power_regulator_disable(power); pm_runtime_put_sync(&power->pdev->dev); return 0; }
Hi,
On Fri, May 20, 2022 at 2:27 PM Kuogee Hsieh quic_khsieh@quicinc.com wrote:
@@ -101,11 +101,6 @@ struct dp_reg_entry { int disable_load; };
-struct dp_regulator_cfg {
int num;struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
You forgot to delete the #define of DP_DEV_REGULATOR_MAX earlier in this file.
Other than that, this looks good to me.
Reviewed-by: Douglas Anderson dianders@chromium.org
dri-devel@lists.freedesktop.org
-
Doug Anderson -
Kuogee Hsieh