| Message ID | 20220422072841.2206452-10-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: RK356x VOP2 support | expand |
On Friday, 22 April 2022 09:28:26 CEST Sascha Hauer wrote: > The RK3568 has HDMI_TX_AVDD0V9 and HDMI_TX_AVDD_1V8 supply inputs needed > for the HDMI port. add support for these to the driver for boards which > have them supplied by switchable regulators. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 41 +++++++++++++++++++-- > 1 file changed, 38 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index > b64cc62c7b5af..fe4f9556239ac 100644 > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > @@ -9,6 +9,7 @@ > #include <linux/platform_device.h> > #include <linux/phy/phy.h> > #include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > > #include <drm/bridge/dw_hdmi.h> > #include <drm/drm_edid.h> > @@ -76,6 +77,8 @@ struct rockchip_hdmi { > struct clk *ref_clk; > struct clk *grf_clk; > struct dw_hdmi *hdmi; > + struct regulator *avdd_0v9; > + struct regulator *avdd_1v8; > struct phy *phy; > }; > > @@ -226,6 +229,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi > *hdmi) return PTR_ERR(hdmi->grf_clk); > } > > + hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9"); > + if (IS_ERR(hdmi->avdd_0v9)) > + return PTR_ERR(hdmi->avdd_0v9); > + > + hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8"); > + if (IS_ERR(hdmi->avdd_1v8)) > + return PTR_ERR(hdmi->avdd_1v8); > + > return 0; > } > > @@ -566,11 +577,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev, > struct device *master, return ret; > } > > + ret = regulator_enable(hdmi->avdd_0v9); > + if (ret) { > + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: %d\n", ret); > + goto err_avdd_0v9; > + } > + > + ret = regulator_enable(hdmi->avdd_1v8); > + if (ret) { > + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: %d\n", ret); > + goto err_avdd_1v8; > + } > + > ret = clk_prepare_enable(hdmi->ref_clk); > if (ret) { > DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI reference clock: %d\n", > ret); > - return ret; > + goto err_clk; > } > > if (hdmi->chip_data == &rk3568_chip_data) { > @@ -594,10 +617,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev, > struct device *master, */ > if (IS_ERR(hdmi->hdmi)) { > ret = PTR_ERR(hdmi->hdmi); > - drm_encoder_cleanup(encoder); > - clk_disable_unprepare(hdmi->ref_clk); > + goto err_bind; > } > > + return 0; > + > +err_bind: > + clk_disable_unprepare(hdmi->ref_clk); > + drm_encoder_cleanup(encoder); > +err_clk: > + regulator_disable(hdmi->avdd_1v8); > +err_avdd_1v8: > + regulator_disable(hdmi->avdd_0v9); > +err_avdd_0v9: > return ret; > } > > @@ -608,6 +640,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, > struct device *master, > > dw_hdmi_unbind(hdmi->hdmi); > clk_disable_unprepare(hdmi->ref_clk); > + > + regulator_disable(hdmi->avdd_1v8); > + regulator_disable(hdmi->avdd_0v9); > } > > static const struct component_ops dw_hdmi_rockchip_ops = { Is it possible to probe for those avdd_0v9 and avdd_1v8 regulators only on devices that should have them? On a Rock64 (rk3328), but probably for all VOP1 devices, they're not present and that results in the following warnings: dwhdmi-rockchip ff3c0000.hdmi: supply avdd-0v9 not found, using dummy regulator dwhdmi-rockchip ff3c0000.hdmi: supply avdd-1v8 not found, using dummy regulator And those get repeated a number of times. In my last 6.10-rc5 boot it was 5 each and in my last 6.10-rc6 boot it was 9 each. It seems the number varies per boot and is not connected to the specific kernel version. In [1] I got 36 "dummy regulator" warnings, but some were from others. [1] https://lore.kernel.org/linux-rockchip/2528743.FZeJfPzi8P@bagend/ Cheers, Diederik
Am Donnerstag, 4. Juli 2024, 11:09:00 CEST schrieb Diederik de Haas: > On Friday, 22 April 2022 09:28:26 CEST Sascha Hauer wrote: > > The RK3568 has HDMI_TX_AVDD0V9 and HDMI_TX_AVDD_1V8 supply inputs needed > > for the HDMI port. add support for these to the driver for boards which > > have them supplied by switchable regulators. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > --- > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 41 +++++++++++++++++++-- > > 1 file changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index > > b64cc62c7b5af..fe4f9556239ac 100644 > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > > @@ -9,6 +9,7 @@ > > #include <linux/platform_device.h> > > #include <linux/phy/phy.h> > > #include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > > > #include <drm/bridge/dw_hdmi.h> > > #include <drm/drm_edid.h> > > @@ -76,6 +77,8 @@ struct rockchip_hdmi { > > struct clk *ref_clk; > > struct clk *grf_clk; > > struct dw_hdmi *hdmi; > > + struct regulator *avdd_0v9; > > + struct regulator *avdd_1v8; > > struct phy *phy; > > }; > > > > @@ -226,6 +229,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi > > *hdmi) return PTR_ERR(hdmi->grf_clk); > > } > > > > + hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9"); > > + if (IS_ERR(hdmi->avdd_0v9)) > > + return PTR_ERR(hdmi->avdd_0v9); > > + > > + hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8"); > > + if (IS_ERR(hdmi->avdd_1v8)) > > + return PTR_ERR(hdmi->avdd_1v8); > > + > > return 0; > > } > > > > @@ -566,11 +577,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev, > > struct device *master, return ret; > > } > > > > + ret = regulator_enable(hdmi->avdd_0v9); > > + if (ret) { > > + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: > %d\n", ret); > > + goto err_avdd_0v9; > > + } > > + > > + ret = regulator_enable(hdmi->avdd_1v8); > > + if (ret) { > > + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: > %d\n", ret); > > + goto err_avdd_1v8; > > + } > > + > > ret = clk_prepare_enable(hdmi->ref_clk); > > if (ret) { > > DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI > reference clock: %d\n", > > ret); > > - return ret; > > + goto err_clk; > > } > > > > if (hdmi->chip_data == &rk3568_chip_data) { > > @@ -594,10 +617,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev, > > struct device *master, */ > > if (IS_ERR(hdmi->hdmi)) { > > ret = PTR_ERR(hdmi->hdmi); > > - drm_encoder_cleanup(encoder); > > - clk_disable_unprepare(hdmi->ref_clk); > > + goto err_bind; > > } > > > > + return 0; > > + > > +err_bind: > > + clk_disable_unprepare(hdmi->ref_clk); > > + drm_encoder_cleanup(encoder); > > +err_clk: > > + regulator_disable(hdmi->avdd_1v8); > > +err_avdd_1v8: > > + regulator_disable(hdmi->avdd_0v9); > > +err_avdd_0v9: > > return ret; > > } > > > > @@ -608,6 +640,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, > > struct device *master, > > > > dw_hdmi_unbind(hdmi->hdmi); > > clk_disable_unprepare(hdmi->ref_clk); > > + > > + regulator_disable(hdmi->avdd_1v8); > > + regulator_disable(hdmi->avdd_0v9); > > } > > > > static const struct component_ops dw_hdmi_rockchip_ops = { > > Is it possible to probe for those avdd_0v9 and avdd_1v8 regulators only on > devices that should have them? > > On a Rock64 (rk3328), but probably for all VOP1 devices, they're not present > and that results in the following warnings: > dwhdmi-rockchip ff3c0000.hdmi: supply avdd-0v9 not found, using dummy regulator > dwhdmi-rockchip ff3c0000.hdmi: supply avdd-1v8 not found, using dummy regulator counter-argument, why not define them in the dts? I.e. looking at the rock64 schematics, you want the dvideo_avdd_1v8 (from LDO1) and dvideo_avdd_1v0 (from LDO3) if I'm not mistaken. Why this stuff is called dvideo and not hdmi in there I have no clue ;-) Heiko
Am 04.07.24 um 11:09 schrieb Diederik de Haas: > On Friday, 22 April 2022 09:28:26 CEST Sascha Hauer wrote: >> The RK3568 has HDMI_TX_AVDD0V9 and HDMI_TX_AVDD_1V8 supply inputs needed >> for the HDMI port. add support for these to the driver for boards which >> have them supplied by switchable regulators. >> >> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> >> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> --- >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 41 +++++++++++++++++++-- >> 1 file changed, 38 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index >> b64cc62c7b5af..fe4f9556239ac 100644 >> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> @@ -9,6 +9,7 @@ >> #include <linux/platform_device.h> >> #include <linux/phy/phy.h> >> #include <linux/regmap.h> >> +#include <linux/regulator/consumer.h> >> >> #include <drm/bridge/dw_hdmi.h> >> #include <drm/drm_edid.h> >> @@ -76,6 +77,8 @@ struct rockchip_hdmi { >> struct clk *ref_clk; >> struct clk *grf_clk; >> struct dw_hdmi *hdmi; >> + struct regulator *avdd_0v9; >> + struct regulator *avdd_1v8; >> struct phy *phy; >> }; >> >> @@ -226,6 +229,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi >> *hdmi) return PTR_ERR(hdmi->grf_clk); >> } >> >> + hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9"); >> + if (IS_ERR(hdmi->avdd_0v9)) >> + return PTR_ERR(hdmi->avdd_0v9); >> + >> + hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8"); >> + if (IS_ERR(hdmi->avdd_1v8)) >> + return PTR_ERR(hdmi->avdd_1v8); >> + >> return 0; >> } >> >> @@ -566,11 +577,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev, >> struct device *master, return ret; >> } >> >> + ret = regulator_enable(hdmi->avdd_0v9); >> + if (ret) { >> + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: > %d\n", ret); >> + goto err_avdd_0v9; >> + } >> + >> + ret = regulator_enable(hdmi->avdd_1v8); >> + if (ret) { >> + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: > %d\n", ret); >> + goto err_avdd_1v8; >> + } >> + >> ret = clk_prepare_enable(hdmi->ref_clk); >> if (ret) { >> DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI > reference clock: %d\n", >> ret); >> - return ret; >> + goto err_clk; >> } >> >> if (hdmi->chip_data == &rk3568_chip_data) { >> @@ -594,10 +617,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev, >> struct device *master, */ >> if (IS_ERR(hdmi->hdmi)) { >> ret = PTR_ERR(hdmi->hdmi); >> - drm_encoder_cleanup(encoder); >> - clk_disable_unprepare(hdmi->ref_clk); >> + goto err_bind; >> } >> >> + return 0; >> + >> +err_bind: >> + clk_disable_unprepare(hdmi->ref_clk); >> + drm_encoder_cleanup(encoder); >> +err_clk: >> + regulator_disable(hdmi->avdd_1v8); >> +err_avdd_1v8: >> + regulator_disable(hdmi->avdd_0v9); >> +err_avdd_0v9: >> return ret; >> } >> >> @@ -608,6 +640,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, >> struct device *master, >> >> dw_hdmi_unbind(hdmi->hdmi); >> clk_disable_unprepare(hdmi->ref_clk); >> + >> + regulator_disable(hdmi->avdd_1v8); >> + regulator_disable(hdmi->avdd_0v9); >> } >> >> static const struct component_ops dw_hdmi_rockchip_ops = { > > Is it possible to probe for those avdd_0v9 and avdd_1v8 regulators only on > devices that should have them? > Those regulators exist for _all_ RK SoCs that use dw-hdmi controller, as it has to be supplied in same why (as it is always the same controller). In particular case of rock64[0] its: DVIDEO_AVDD_1V8P6 -> VCC_18 DVIDEO_AVDD_1V0M6 -> VDD_10 So: Just fix the device tree and your warnings are gone :) [0] https://files.pine64.org/doc/rock64/ROCK64_Schematic_v2.0_20171019.pdf > On a Rock64 (rk3328), but probably for all VOP1 devices, they're not present > and that results in the following warnings: > dwhdmi-rockchip ff3c0000.hdmi: supply avdd-0v9 not found, using dummy regulator > dwhdmi-rockchip ff3c0000.hdmi: supply avdd-1v8 not found, using dummy regulator > > And those get repeated a number of times. In my last 6.10-rc5 boot it was 5 > each and in my last 6.10-rc6 boot it was 9 each. It seems the number varies > per boot and is not connected to the specific kernel version. > > In [1] I got 36 "dummy regulator" warnings, but some were from others. > > [1] https://lore.kernel.org/linux-rockchip/2528743.FZeJfPzi8P@bagend/ > > Cheers, > Diederik > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On Thursday, 4 July 2024 12:00:43 CEST Heiko Stübner wrote: > > Is it possible to probe for those avdd_0v9 and avdd_1v8 regulators only on > > devices that should have them? > > > > On a Rock64 (rk3328), but probably for all VOP1 devices, they're not > > present and that results in the following warnings: > > dwhdmi-rockchip ff3c0000.hdmi: supply avdd-0v9 not found, using dummy > > regulator dwhdmi-rockchip ff3c0000.hdmi: supply avdd-1v8 not found, using > > dummy regulator > counter-argument, why not define them in the dts? > I.e. looking at the rock64 schematics, you want the dvideo_avdd_1v8 (from > LDO1) and dvideo_avdd_1v0 (from LDO3) if I'm not mistaken. Why this stuff > is called dvideo and not hdmi in there I have no clue ;-) That appears to be a valid counter-argument ... On donderdag 4 juli 2024 12:28:53 CEST Alex Bee wrote: > Those regulators exist for _all_ RK SoCs that use dw-hdmi controller, as it > has to be supplied in same why (as it is always the same controller). In > particular case of rock64[0] its: > > DVIDEO_AVDD_1V8P6 -> VCC_18 > DVIDEO_AVDD_1V0M6 -> VDD_10 > > So: Just fix the device tree and your warnings are gone :) > > [0] https://files.pine64.org/doc/rock64/ROCK64_Schematic_v2.0_20171019.pdf ... confirmed by Alex Bee. I do wonder about 0.9V vs 1.0V, but I'll bug someone else about that ;-) Cheers, Diederik
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index b64cc62c7b5af..fe4f9556239ac 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -9,6 +9,7 @@ #include <linux/platform_device.h> #include <linux/phy/phy.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h> #include <drm/bridge/dw_hdmi.h> #include <drm/drm_edid.h> @@ -76,6 +77,8 @@ struct rockchip_hdmi { struct clk *ref_clk; struct clk *grf_clk; struct dw_hdmi *hdmi; + struct regulator *avdd_0v9; + struct regulator *avdd_1v8; struct phy *phy; }; @@ -226,6 +229,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) return PTR_ERR(hdmi->grf_clk); } + hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9"); + if (IS_ERR(hdmi->avdd_0v9)) + return PTR_ERR(hdmi->avdd_0v9); + + hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8"); + if (IS_ERR(hdmi->avdd_1v8)) + return PTR_ERR(hdmi->avdd_1v8); + return 0; } @@ -566,11 +577,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master, return ret; } + ret = regulator_enable(hdmi->avdd_0v9); + if (ret) { + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: %d\n", ret); + goto err_avdd_0v9; + } + + ret = regulator_enable(hdmi->avdd_1v8); + if (ret) { + DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: %d\n", ret); + goto err_avdd_1v8; + } + ret = clk_prepare_enable(hdmi->ref_clk); if (ret) { DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI reference clock: %d\n", ret); - return ret; + goto err_clk; } if (hdmi->chip_data == &rk3568_chip_data) { @@ -594,10 +617,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master, */ if (IS_ERR(hdmi->hdmi)) { ret = PTR_ERR(hdmi->hdmi); - drm_encoder_cleanup(encoder); - clk_disable_unprepare(hdmi->ref_clk); + goto err_bind; } + return 0; + +err_bind: + clk_disable_unprepare(hdmi->ref_clk); + drm_encoder_cleanup(encoder); +err_clk: + regulator_disable(hdmi->avdd_1v8); +err_avdd_1v8: + regulator_disable(hdmi->avdd_0v9); +err_avdd_0v9: return ret; } @@ -608,6 +640,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master, dw_hdmi_unbind(hdmi->hdmi); clk_disable_unprepare(hdmi->ref_clk); + + regulator_disable(hdmi->avdd_1v8); + regulator_disable(hdmi->avdd_0v9); } static const struct component_ops dw_hdmi_rockchip_ops = {