Re: [PATCH v16 0/3] eDP/DP Phy vdda realted function

[Johan Hovold <johan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3002 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]