| Message ID | 1657038556-2231-1-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | eDP/DP Phy vdda realted function | expand |
On 05-07-22, 09:29, Kuogee Hsieh wrote: > 0) rebase on https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tree > 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 Applied, thanks
On Tue, Jul 05, 2022 at 09:29:13AM -0700, Kuogee Hsieh wrote: > 0) rebase on https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tree > 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 | 8 --- > drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------ > drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++ > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 ++++++++++--- > 5 files changed, 46 insertions(+), 124 deletions(-) This series breaks USB and PCIe for some SC8280XP and SA540P machines where the DP PHY regulators are shared with other PHYs whose drivers do not request a load. Specifically, the hard-coded vdda-phy load of 21.8 mA added by this series, causes several RPMh regulators to now be put in low-power mode. I found Doug's suggestion to handle situations like this in regulator core: https://lore.kernel.org/all/20180814170617.100087-1-dianders@chromium.org/ but since that was rejected, how do we deal with this generally? In the above thread Doug mentioned adding support for load requests to further drivers and Bjorn mentioned working around it by adding regulator-system-load properties to DT. It seems quite likely that changes like this one affects other systems too, and the effects may be hard to debug. So a more general solution than playing whack-a-mole using DT would be good to have. Johan
On Thu, Jul 21, 2022 at 12:31:41PM +0200, Johan Hovold wrote: If you're copying someone into a thread that's not obviously relevant for them it's good practice to put a note about it at the top of the mail to reduce the chances that it just gets deleted unread - people get copies of all sorts of random stuff for not great reasons (like getting pulled in by checkpatch due to once having done a cleanup) and are often quicky to delete things. > This series breaks USB and PCIe for some SC8280XP and SA540P machines > where the DP PHY regulators are shared with other PHYs whose drivers do > not request a load. > Specifically, the hard-coded vdda-phy load of 21.8 mA added by this > series, causes several RPMh regulators to now be put in low-power mode. > I found Doug's suggestion to handle situations like this in regulator > core: > > https://lore.kernel.org/all/20180814170617.100087-1-dianders@chromium.org/ > but since that was rejected, how do we deal with this generally? > In the above thread Doug mentioned adding support for load requests to > further drivers and Bjorn mentioned working around it by adding > regulator-system-load properties to DT. > It seems quite likely that changes like this one affects other systems > too, and the effects may be hard to debug. So a more general solution > than playing whack-a-mole using DT would be good to have. You could add a way to specify constant base loads in DT on either a per regulator or per consumer basis.
On Thu, Jul 21, 2022 at 12:20:31PM +0100, Mark Brown wrote: > You could add a way to specify constant base loads in DT on either a per > regulator or per consumer basis. ...and also note that this is only an issue if the system gives permission to change the mode in the constraints which is pretty rare.
On Thu, Jul 21, 2022 at 12:56:37PM +0100, Mark Brown wrote: > On Thu, Jul 21, 2022 at 12:20:31PM +0100, Mark Brown wrote: > > > You could add a way to specify constant base loads in DT on either a per > > regulator or per consumer basis. > > ...and also note that this is only an issue if the system gives > permission to change the mode in the constraints which is pretty rare. Yeah, apparently only Qualcomm is using regulator-allow-set-load at the moment, but it seems pretty common there. We should probably just drop that from the platforms affected by this particular regression and perhaps later add it back where it makes sense (e.g. after making sure all consumers specify a load in some way). Johan
On Thu, 21 Jul 2022 at 13:31, Johan Hovold <johan@kernel.org> wrote: > > On Tue, Jul 05, 2022 at 09:29:13AM -0700, Kuogee Hsieh wrote: > > 0) rebase on https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tree > > 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 | 8 --- > > drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------ > > drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++ > > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 ++++++++++--- > > 5 files changed, 46 insertions(+), 124 deletions(-) > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > where the DP PHY regulators are shared with other PHYs whose drivers do > not request a load. I'm trying to understand, why does this series cause the regression. Previously it would be the DP controller voting on the regulators load, now it has been moved to the DP/eDP PHYs. > Specifically, the hard-coded vdda-phy load of 21.8 mA added by this > series, causes several RPMh regulators to now be put in low-power mode. > > I found Doug's suggestion to handle situations like this in regulator > core: > > https://lore.kernel.org/all/20180814170617.100087-1-dianders@chromium.org/ > > but since that was rejected, how do we deal with this generally? > > In the above thread Doug mentioned adding support for load requests to > further drivers and Bjorn mentioned working around it by adding > regulator-system-load properties to DT. > > It seems quite likely that changes like this one affects other systems > too, and the effects may be hard to debug. So a more general solution > than playing whack-a-mole using DT would be good to have. I think enabling a regulator which supports set_load() and without load being set should cause a warning, at least with CONFIG_REGULATOR_DEBUG. WDYT?
Hi, On Thu, Jul 21, 2022 at 4:20 AM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jul 21, 2022 at 12:31:41PM +0200, Johan Hovold wrote: > > If you're copying someone into a thread that's not obviously relevant > for them it's good practice to put a note about it at the top of the > mail to reduce the chances that it just gets deleted unread - people get > copies of all sorts of random stuff for not great reasons (like getting > pulled in by checkpatch due to once having done a cleanup) and are often > quicky to delete things. > > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > > where the DP PHY regulators are shared with other PHYs whose drivers do > > not request a load. > > > Specifically, the hard-coded vdda-phy load of 21.8 mA added by this > > series, causes several RPMh regulators to now be put in low-power mode. > > > I found Doug's suggestion to handle situations like this in regulator > > core: > > > > https://lore.kernel.org/all/20180814170617.100087-1-dianders@chromium.org/ > > > but since that was rejected, how do we deal with this generally? > > > In the above thread Doug mentioned adding support for load requests to > > further drivers and Bjorn mentioned working around it by adding > > regulator-system-load properties to DT. > > > It seems quite likely that changes like this one affects other systems > > too, and the effects may be hard to debug. So a more general solution > > than playing whack-a-mole using DT would be good to have. > > You could add a way to specify constant base loads in DT on either a per > regulator or per consumer basis. Yes, this please! ...on a per consumer basis. :-) It's been on my wishlist for a while and would eliminate a massive amount of code / tables in the drivers. We could debate syntax, but I guess you'd either do it w/ two cells vdda-phy-supply = <&vdda_mipi_dsi0_1p2 21800>; ...or with matching properties: vdda-phy-supply = <&vdda_mipi_dsi0_1p2>; vdda-phy-supply-base-load = <21800>; -Doug
Hi, On Thu, Jul 21, 2022 at 6:25 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > > where the DP PHY regulators are shared with other PHYs whose drivers do > > not request a load. > > I'm trying to understand, why does this series cause the regression. > Previously it would be the DP controller voting on the regulators > load, now it has been moved to the DP/eDP PHYs. I think in the past not many device trees actually hooked up the regulators to the DP/eDP but did hook up the regulators to the PHYs? That means they didn't used to get a regulator_set_load() on them and now they do? It should also be noted that we're now setting the load for a bunch of USB PHYs that we didn't used to set a load on... > > It seems quite likely that changes like this one affects other systems > > too, and the effects may be hard to debug. So a more general solution > > than playing whack-a-mole using DT would be good to have. > > I think enabling a regulator which supports set_load() and without > load being set should cause a warning, at least with > CONFIG_REGULATOR_DEBUG. WDYT? I'm not a total fan. I'd love to see evidence to the contrary, but I'm a believer that the amount of extra cruft involved with all these regulator_set_load() calls is overkill for most cases. All the extra code / per-SoC tables added to drivers isn't ideal. Every single LDO on Qualcomm's PMICs seems to be able to be set in "high power mode" and "low power mode", but I think the majority of clients only really care about two things: on and in high power mode vs. off. I think the amount of stuff peripherals can do in low power mode is super limited, so you have to be _really_ sure that the peripheral won't draw too much current without you having a chance to reconfigure the regulator. Of course, if the load could be easily specified in the device tree and we could get rid of all the cruft in the drivers then maybe that would be OK. -Doug
Hi, On Thu, Jul 21, 2022 at 7:39 AM Doug Anderson <dianders@chromium.org> wrote: > > > You could add a way to specify constant base loads in DT on either a per > > regulator or per consumer basis. > > Yes, this please! ...on a per consumer basis. :-) It's been on my > wishlist for a while and would eliminate a massive amount of code / > tables in the drivers. > > We could debate syntax, but I guess you'd either do it w/ two cells > > vdda-phy-supply = <&vdda_mipi_dsi0_1p2 21800>; > > ...or with matching properties: > > vdda-phy-supply = <&vdda_mipi_dsi0_1p2>; > vdda-phy-supply-base-load = <21800>; Ah, sorry to respond to my own thread so quickly, but I just thought of a reason for the "matching properties" solution instead of the two cells. It would allow the SoC "dtsi" file to specify a base load while the board "dts" file can then specify the supply. That feels like it could be a nice solution. -Doug
On Thu, Jul 21, 2022 at 07:49:55AM -0700, Doug Anderson wrote: > Every single LDO on Qualcomm's PMICs seems to be able to be set in > "high power mode" and "low power mode", but I think the majority of > clients only really care about two things: on and in high power mode > vs. off. I think the amount of stuff peripherals can do in low power > mode is super limited, so you have to be _really_ sure that the > peripheral won't draw too much current without you having a chance to > reconfigure the regulator. *Generally* a low power mode would be mainly useful for low power retention type states, not active use.
Hi, On Thu, Jul 21, 2022 at 8:06 AM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jul 21, 2022 at 07:49:55AM -0700, Doug Anderson wrote: > > > Every single LDO on Qualcomm's PMICs seems to be able to be set in > > "high power mode" and "low power mode", but I think the majority of > > clients only really care about two things: on and in high power mode > > vs. off. I think the amount of stuff peripherals can do in low power > > mode is super limited, so you have to be _really_ sure that the > > peripheral won't draw too much current without you having a chance to > > reconfigure the regulator. > > *Generally* a low power mode would be mainly useful for low power > retention type states, not active use. Right. Certainly the case I've seen where it is most useful is in S3 where we need to keep a device powered just enough to detect a wakeup, but it can definitely also be useful for mostly idle devices that we need to keep powered to retain memory so they can start up again quickly. I guess I'd put it this way, though: how many drivers in Linux today have _two_ calls to regulator_set_load(): one for the "active" state and one for the retention state. Looks like UFS maybe. Any others? For most devices the pattern is: * get all of our regulators. * for each regulator, set the load to something that will trigger HPM when we're using the regulator. * turn regulators on when we need power and off when we don't. All the extra scaffolding and tables to pass something to regulator_set_load() is just a lot of noise to add for drivers that don't have any concept of "retention" mode and don't need it. -Doug
On Thu, Jul 21, 2022 at 08:43:48AM -0700, Doug Anderson wrote: > I guess I'd put it this way, though: how many drivers in Linux today > have _two_ calls to regulator_set_load(): one for the "active" state > and one for the retention state. Looks like UFS maybe. Any others? For > most devices the pattern is: Oh, I'm not saying there's sensible implementations in drivers. In general I'd say that as with voltage if a driver is not actively managing the load during runtime it should not be setting it at all, one time configuration should be left to the constraints if it's needed.
On Thu, Jul 21, 2022 at 07:52:01AM -0700, Doug Anderson wrote: > On Thu, Jul 21, 2022 at 7:39 AM Doug Anderson <dianders@chromium.org> wrote: > > vdda-phy-supply = <&vdda_mipi_dsi0_1p2>; > > vdda-phy-supply-base-load = <21800>; > Ah, sorry to respond to my own thread so quickly, but I just thought > of a reason for the "matching properties" solution instead of the two > cells. It would allow the SoC "dtsi" file to specify a base load while > the board "dts" file can then specify the supply. That feels like it > could be a nice solution. For a per device thing it probably shouldn't be called a "base load", it's just the load. I would question how useful this would be, aside from the concerns vendors will likely have with disclosing actual power consumptions if the number never changes then it is immaterial which device contributes which milliamp of load, you just want to configure a fixed mode on the regulator and have done with it.
On Thu, 21 Jul 2022 at 17:50, Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Jul 21, 2022 at 6:25 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > > > where the DP PHY regulators are shared with other PHYs whose drivers do > > > not request a load. > > > > I'm trying to understand, why does this series cause the regression. > > Previously it would be the DP controller voting on the regulators > > load, now it has been moved to the DP/eDP PHYs. > > I think in the past not many device trees actually hooked up the > regulators to the DP/eDP but did hook up the regulators to the PHYs? > That means they didn't used to get a regulator_set_load() on them and > now they do? > > It should also be noted that we're now setting the load for a bunch of > USB PHYs that we didn't used to set a load on... Might be the case, yes. > > > It seems quite likely that changes like this one affects other systems > > > too, and the effects may be hard to debug. So a more general solution > > > than playing whack-a-mole using DT would be good to have. > > > > I think enabling a regulator which supports set_load() and without > > load being set should cause a warning, at least with > > CONFIG_REGULATOR_DEBUG. WDYT? > > I'm not a total fan. I'd love to see evidence to the contrary, but I'm > a believer that the amount of extra cruft involved with all these > regulator_set_load() calls is overkill for most cases. All the extra > code / per-SoC tables added to drivers isn't ideal. I'm fine with the load being specified in the DT (and that sounds like a good idea for me too). What I'd like to achieve is catching load-enabled regulators for which we did not set the load (via an API call or via the DT).
Hi, On Thu, Jul 21, 2022 at 7:52 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Jul 21, 2022 at 7:39 AM Doug Anderson <dianders@chromium.org> wrote: > > > > > You could add a way to specify constant base loads in DT on either a per > > > regulator or per consumer basis. > > > > Yes, this please! ...on a per consumer basis. :-) It's been on my > > wishlist for a while and would eliminate a massive amount of code / > > tables in the drivers. > > > > We could debate syntax, but I guess you'd either do it w/ two cells > > > > vdda-phy-supply = <&vdda_mipi_dsi0_1p2 21800>; > > > > ...or with matching properties: > > > > vdda-phy-supply = <&vdda_mipi_dsi0_1p2>; > > vdda-phy-supply-base-load = <21800>; > > Ah, sorry to respond to my own thread so quickly, but I just thought > of a reason for the "matching properties" solution instead of the two > cells. It would allow the SoC "dtsi" file to specify a base load while > the board "dts" file can then specify the supply. That feels like it > could be a nice solution. This seemed easy, so I whipped up a quick prototype. Forewarned that I did very little detailed testing. I didn't CC everyone on this thread, but hopefully folks can find it if they are interested... https://lore.kernel.org/r/20220721182622.RFC.1.I8a64b707169cfd73d9309c5eaf5d43b8bc4db988@changeid -Doug
Sorry about the late follow-up on this. Has been one of those constant-preemption weeks. And thanks Doug and Mark for the insightful comments in this and the dt-load RFC thread. On Thu, Jul 21, 2022 at 07:49:55AM -0700, Doug Anderson wrote: > Hi, > > On Thu, Jul 21, 2022 at 6:25 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > > This series breaks USB and PCIe for some SC8280XP and SA540P machines > > > where the DP PHY regulators are shared with other PHYs whose drivers do > > > not request a load. > > > > I'm trying to understand, why does this series cause the regression. > > Previously it would be the DP controller voting on the regulators > > load, now it has been moved to the DP/eDP PHYs. > > I think in the past not many device trees actually hooked up the > regulators to the DP/eDP but did hook up the regulators to the PHYs? > That means they didn't used to get a regulator_set_load() on them and > now they do? This was the case for us, but this could probably also partly be blamed on the DP stuff being work-in-progress and the regulators were only ever specified in the PHY nodes. > It should also be noted that we're now setting the load for a bunch of > USB PHYs that we didn't used to set a load on... That was my concern. But after skimming the mainline dts it seems that no users of these drivers are currently allowing the regulator mode to be changed for those particular regulators. Well, except for the sc8280xp (derivate) machines in -next that I'm working on and that regressed of course. I guess we just need to drop all those regulator-allow-set-load properties for now even if using DT for power-management configuration this way does seem to run against the whole DT-as-hardware-description idea (e.g. we may want to add them back when/if active- and idle loads are specified by the corresponding Linux drivers). > > > It seems quite likely that changes like this one affects other systems > > > too, and the effects may be hard to debug. So a more general solution > > > than playing whack-a-mole using DT would be good to have. > > > > I think enabling a regulator which supports set_load() and without > > load being set should cause a warning, at least with > > CONFIG_REGULATOR_DEBUG. WDYT? > > I'm not a total fan. I'd love to see evidence to the contrary, but I'm > a believer that the amount of extra cruft involved with all these > regulator_set_load() calls is overkill for most cases. All the extra > code / per-SoC tables added to drivers isn't ideal. > > Every single LDO on Qualcomm's PMICs seems to be able to be set in > "high power mode" and "low power mode", but I think the majority of > clients only really care about two things: on and in high power mode > vs. off. I think the amount of stuff peripherals can do in low power > mode is super limited, so you have to be _really_ sure that the > peripheral won't draw too much current without you having a chance to > reconfigure the regulator. > > Of course, if the load could be easily specified in the device tree > and we could get rid of all the cruft in the drivers then maybe that > would be OK. It seems a decision was made against this (which I tend to agree with), and that instead it is now easier for drivers to specify loads using your updated bulk-regulator implementation. But that doesn't address the problem that was trying to highlight here, and that you had noticed years ago, namely that using set_load only works reliably if *all* consumers use it. Otherwise we may end up with a couple of drivers adding some 10 mA each to the load, while other driver do not specify a load so that while the actual load is really above the 30 mA HPM threshold, we still end up in LPM. Shouldn't an enabled regulator from a consumer that didn't specify a load somehow result in HPM always being selected (e.g. count as INT_MAX load as Doug suggested some years ago)? Note that LPM may be ok until that last consumer that didn't specify a load pushes it over the HPM threshold (so specifying a regulator base load in DT would also prevent ever selecting LPM). This case of mixing consumers that specify and don't specify loads might at least warrant a warning otherwise to avoid ending up in a hard-to-debug situation with failing USB and PCIe due to a seemingly unrelated DP PHY update. At some point in the discussion I thought Mark suggested removing set_load from drivers that don't actually manage active and idle loads. That would also work, at least until the day one of the drivers adds support for idle loads. Johan
On Fri, Jul 29, 2022 at 03:35:33PM +0200, Johan Hovold wrote: > I guess we just need to drop all those regulator-allow-set-load > properties for now even if using DT for power-management configuration > this way does seem to run against the whole DT-as-hardware-description > idea (e.g. we may want to add them back when/if active- and idle loads > are specified by the corresponding Linux drivers). Well, there's also a question of if the hardware can usefully make use of the facility - is there any non-suspend state where the regulator needs to be on but is drawing so little current that it's worth trying to select a lower power mode? > But that doesn't address the problem that was trying to highlight here, > and that you had noticed years ago, namely that using set_load only > works reliably if *all* consumers use it. > Shouldn't an enabled regulator from a consumer that didn't specify a > load somehow result in HPM always being selected (e.g. count as INT_MAX > load as Doug suggested some years ago)? Possibly, but note that as well as the consumers with software drivers you also have to consider any passive consumers on the board which may not have any representation in DT so the actual numbers may well be off even if every consumer is trying to keep things up to date. You also come back to the "let's just shove a random number in here" problem. For ultimate saftey we probably want a command line option to gate the feature which people can set to say they've audited their full software/hardware integration stack. > At some point in the discussion I thought Mark suggested removing > set_load from drivers that don't actually manage active and idle loads. > That would also work, at least until the day one of the drivers adds > support for idle loads. Yes, if the driver isn't actively managing loads it's probably not doing anything useful. The difficulties with this sort of system integration question is an unfortunate consequence of DT, having to describe what's safe for an unknown software stack is fundamentally hard. I do question how much effort it's worth putting into enabling this, especially in cases where the regulator is shared - how much power is actually saved in the grand scheme of things given that this is only taking effect when the system is out of suspend and we tend to be talking about some percentage of the power being drawn on something which is presumably already consuming very little power for this to be at all relevant?
On Fri, Jul 29, 2022 at 03:07:47PM +0100, Mark Brown wrote: > On Fri, Jul 29, 2022 at 03:35:33PM +0200, Johan Hovold wrote: > > > I guess we just need to drop all those regulator-allow-set-load > > properties for now even if using DT for power-management configuration > > this way does seem to run against the whole DT-as-hardware-description > > idea (e.g. we may want to add them back when/if active- and idle loads > > are specified by the corresponding Linux drivers). > > Well, there's also a question of if the hardware can usefully make use > of the facility - is there any non-suspend state where the regulator > needs to be on but is drawing so little current that it's worth trying > to select a lower power mode? Good point. > > But that doesn't address the problem that was trying to highlight here, > > and that you had noticed years ago, namely that using set_load only > > works reliably if *all* consumers use it. > > > Shouldn't an enabled regulator from a consumer that didn't specify a > > load somehow result in HPM always being selected (e.g. count as INT_MAX > > load as Doug suggested some years ago)? > > Possibly, but note that as well as the consumers with software drivers > you also have to consider any passive consumers on the board which may > not have any representation in DT so the actual numbers may well be off > even if every consumer is trying to keep things up to date. You also > come back to the "let's just shove a random number in here" problem. Right, but some of that could be captured in DT with 'regulator-system-load'. > For ultimate saftey we probably want a command line option to gate the > feature which people can set to say they've audited their full > software/hardware integration stack. That sounds like it could be useful. > > At some point in the discussion I thought Mark suggested removing > > set_load from drivers that don't actually manage active and idle loads. > > That would also work, at least until the day one of the drivers adds > > support for idle loads. > > Yes, if the driver isn't actively managing loads it's probably not doing > anything useful. Ok, thanks for confirming. Perhaps we should drop the set_loads() added to the PHY driver by this series then. > The difficulties with this sort of system integration question is an > unfortunate consequence of DT, having to describe what's safe for an > unknown software stack is fundamentally hard. I do question how much > effort it's worth putting into enabling this, especially in cases where > the regulator is shared - how much power is actually saved in the grand > scheme of things given that this is only taking effect when the system > is out of suspend and we tend to be talking about some percentage of the > power being drawn on something which is presumably already consuming > very little power for this to be at all relevant? I tend to agree. Thanks again for your input! Johan
0) rebase on https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git tree 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 | 8 --- drivers/gpu/drm/msm/dp/dp_power.c | 95 +------------------------------ drivers/phy/qualcomm/phy-qcom-edp.c | 12 ++++ drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 ++++++++++--- 5 files changed, 46 insertions(+), 124 deletions(-)