Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 3 Aug 2022 10:50:38 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Mark Brown <broonie@kernel.org>
Cc:     Doug Anderson <dianders@chromium.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Kuogee Hsieh <quic_khsieh@quicinc.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Vinod Koul <vkoul@kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
        Stephen Boyd <swboyd@chromium.org>,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>,
        Andy Gross <agross@kernel.org>,
        "Abhinav Kumar (QUIC)" <quic_abhinavk@quicinc.com>,
        "Aravind Venkateswaran (QUIC)" <quic_aravindh@quicinc.com>,
        Sankeerth Billakanti <quic_sbillaka@quicinc.com>,
        freedreno <freedreno@lists.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Rob Herring <robh@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Subject: Re: [PATCH v16 0/3] eDP/DP Phy vdda realted function
Message-ID: <Yuo23sdBe6tI7g5K@hovoldconsulting.com>
References: <1657038556-2231-1-git-send-email-quic_khsieh@quicinc.com>
 <YtkrDcjTGhpaU1e0@hovoldconsulting.com>
 <CAA8EJprQnnWjDZJy9+zUBsVQCi3jtc0Ngtzzk9MXpwOvuAS68g@mail.gmail.com>
 <CAD=FV=W0m-x9JC=5hQ3urSNmUp8sY-u8YkNd66yrKfRNAH4rcg@mail.gmail.com>
 <YuPiJWQ1/wQbkvD8@hovoldconsulting.com>
 <YuPps+cvVAMugWmy@sirena.org.uk>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="YTsIOlOo8860ehFK"
Content-Disposition: inline
In-Reply-To: <YuPps+cvVAMugWmy@sirena.org.uk>
Precedence: bulk


--YTsIOlOo8860ehFK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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:
>=20
> > 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).
>=20
> 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.
>=20
> > 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)?
>=20
> 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.
=20
> > 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.
>=20
> 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.
=20
> 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

--YTsIOlOo8860ehFK
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQQHbPq+cpGvN/peuzMLxc3C7H1lCAUCYuo22gAKCRALxc3C7H1l
CArBAP4+W07nkP4v5zuEdV1LepVpAWJtpTTVnGwdfkVwnr5FAgD+I8xjWidPc2Kq
nD+BAXuccWyyiOPlBuyM2GD/ZXjL1wQ=
=ehiB
-----END PGP SIGNATURE-----

--YTsIOlOo8860ehFK--