Date:   Fri, 29 Jul 2022 15:07:47 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Johan Hovold <johan@...nel.org>
Cc:     Doug Anderson <dianders@...omium.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Kuogee Hsieh <quic_khsieh@...cinc.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Vinod Koul <vkoul@...nel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        Stephen Boyd <swboyd@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Andy Gross <agross@...nel.org>,
        "Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>,
        "Aravind Venkateswaran (QUIC)" <quic_aravindh@...cinc.com>,
        Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
        freedreno <freedreno@...ts.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v16 0/3] eDP/DP Phy vdda realted function

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?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists