Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programmatic access to remote power measurements #51

Merged
merged 8 commits into from
Nov 28, 2023

Conversation

andersson
Copy link
Collaborator

The CDB Assist backend allows the user to request status updates, which includes voltage and current provided to the board.
By formalizing the structure of these updates, generate them at a fixed interval, and redirect them to a fifo for programmatic consumption we can make this information useful to 3rd party tools (e.g. for validation, analysis, or graphing).

The QcomLT Debugboard has a voltage and current sensor, so the same functionality is extended to support this backend natively as well.

Support for invoking an external "status command" is also introduced. This allow board-specific custom measurement mechanisms, such as reading voltage and current sensors from iio, custom USB/network protocols etc.

status.c Fixed Show fixed Hide fixed
status.c Fixed Show fixed Hide fixed
@andersson andersson force-pushed the for-linux-msm/status-support branch from a443508 to b30c5dc Compare November 6, 2023 04:47
@lumag
Copy link
Collaborator

lumag commented Nov 6, 2023

forkpty might require adding -lutil

README Show resolved Hide resolved
@calebccff
Copy link
Member

What sort of data do we want to collect here? If we want to measure power consumption in various states (under load, in suspend, etc), then taking instantaneous measurements of voltage/current and multiplying them feels like a bit of a trap where we could easily end up with very innacurate numbers due to how the values are measured by the hardware.

The qcom_dbg board uses an INA223 which also has a built-in accumulator and can take power measurements directly, so shouldn't we make use of this instead?

@andersson
Copy link
Collaborator Author

forkpty might require adding -lutil

Per the CI this turns out to be required for Debian. I need to figure out why dependency('util') doesn't work on Arch though...

@andersson
Copy link
Collaborator Author

What sort of data do we want to collect here? If we want to measure power consumption in various states (under load, in suspend, etc), then taking instantaneous measurements of voltage/current and multiplying them feels like a bit of a trap where we could easily end up with very innacurate numbers due to how the values are measured by the hardware.

I'm looking for data showing the state of the hardware, for the purpose of getting insights during e.g. suspend. I am not interested in accurate measurements over some time. As you're suggesting, the consumption might fluctuate and we will never be sampling fast enough for that.

The qcom_dbg board uses an INA223 which also has a built-in accumulator and can take power measurements directly, so shouldn't we make use of this instead?

We should indeed be able to rewrite the firmware on the qcomlt dbg board to periodically report the accumulated measurements. It does however currently report mV and mA values. The CDB Assist also report snapshots of voltage and current, and my custom external status-command does the same...

That said, as proposed here, we could add additional unit of measurements, and once you've updated the firmware of the QcomLT dbg board you can report the accumulated values - using the same mechanism.

@andersson andersson force-pushed the for-linux-msm/status-support branch 3 times, most recently from 85d1893 to bbe67f7 Compare November 6, 2023 16:30
device.c Outdated Show resolved Hide resolved
README Show resolved Hide resolved
README Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
cdb_assist.c Show resolved Hide resolved
@andersson andersson force-pushed the for-linux-msm/status-support branch from bbe67f7 to 2549fd1 Compare November 9, 2023 20:24
@lumag lumag enabled auto-merge November 20, 2023 23:45
@lumag
Copy link
Collaborator

lumag commented Nov 20, 2023

@andersson

Merging is blocked
The base branch requires all commits to be signed.

The current format of the cdb_assist backend status messages was never
intended to be parsed by a non-human, and the format doesn't extend well
to other backends.

Extend the format to be formatted as a json string, with a timestamp,
implemented in a separate status module.

The button status is skipped from the new status message, as it reflect
the output-values as set by cdba itself, rather than any measured state.

The timestamp is offset to the first measurement, to avoid disclosing
the uptime of the machine cdba-server is running on.

Signed-off-by: Bjorn Andersson <[email protected]>
The MSG_STATUS_UPDATE message when sent from the client to the server,
has until now indicated a request for a single status update. This was
suitable when the update was for human consumption on occasional basis.

But with the transition to producing status messages for programatical
consumption it makes more sense to just keep the stream on, without the
client having to poll the server for updates. This can in particular
keep the external "status command" mechanism simpler.

Just enabling the stream of status updates would spam the console with
status updates for those users that hasn't updated their clients, so the
client needs to opt-in to the status stream.

Repurpose the status update message (sent from the client) to indicate
that the streaming of status updates should be enabled.

Signed-off-by: Bjorn Andersson <[email protected]>
The updated format and behavior of the status messages is not useful for
human consumption in the console stream, but rather for programatical
consumption by 3rd party tools.

Switch the status message stream to an optional fifo, which can be
consumed separately from the console - by humans or by tools.

The fifo is created and opened and status messages requested, if the
'-s <file>' option is specified.

Signed-off-by: Bjorn Andersson <[email protected]>
The QcomLT DebugBoard has a voltage and current sensor on the 12V DC
line, which the firmware will measure upon request.

Add a periodic request for this information, parse out the responses and
report using the status helper functions.

Signed-off-by: Bjorn Andersson <[email protected]>
While the CDB Assist and QcomLT Debugboard is measuring voltage and
current on the DC jack directly in the backend, other backends and
further measurements can be performed using custom tooling.

Introduce support for invoking an external "status-cmd" to produce such
status updates.

The status command should on its stdout produce json-formatted status
updates following the same format as the cdba-server:

  {"ts":%d.%03d, "name": {["mv"|"ma"]: %u}(, "name2": {["mv"|"ma"]: %u})*}

with the ts aquired using clock_gettime(CLOCK_MONOTONIC) and provided in
seconds and milliseconds since the first measurement.

Signed-off-by: Bjorn Andersson <[email protected]>
Leaving USB connected might cause hickups on the USB bus, and causes
power measurements to report incorrect data. Turn it off once the image
has been downloaded.

Signed-off-by: Bjorn Andersson <[email protected]>
Introduce description of the status fifo, the status command, and the
data format for these in the README.

Signed-off-by: Bjorn Andersson <[email protected]>
@andersson andersson force-pushed the for-linux-msm/status-support branch from 2549fd1 to 8e80245 Compare November 28, 2023 03:21
Provide a sample defining the status-cmd property.

Signed-off-by: Bjorn Andersson <[email protected]>
@andersson andersson force-pushed the for-linux-msm/status-support branch from 8e80245 to 0ad34da Compare November 28, 2023 03:33
@lumag lumag merged commit 3269b8b into linux-msm:master Nov 28, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants