Fix low voltage state regression on 6.18#7256
Draft
popcornmix wants to merge 8 commits intoraspberrypi:rpi-6.18.yfrom
Draft
Fix low voltage state regression on 6.18#7256popcornmix wants to merge 8 commits intoraspberrypi:rpi-6.18.yfrom
popcornmix wants to merge 8 commits intoraspberrypi:rpi-6.18.yfrom
Conversation
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't actually power off the clock. To achieve meaningful power savings, the clock rate must be set to the minimum before disabling. This might be fixed in future firmware releases. Rather than pushing rate management to clock consumers, handle it directly in the clock framework's prepare/unprepare callbacks. In unprepare, set the rate to the firmware-reported minimum before disabling the clock. In prepare, for clocks marked with `maximize` (currently v3d), restore the rate to the maximum after enabling. Signed-off-by: Maíra Canal <mcanal@igalia.com>
If PIXEL_CLK or HEVC_CLK is disabled during boot, the firmware will skip HSM initialization, which would result in a bus lockup. However, those clocks are consumed by drivers (vc4 and HEVC decoder drivers, respectively), which means that they can be enabled/disabled by the drivers. Mark those clocks as CLK_IGNORE_UNUSED to allow them to be disabled by drivers when appropriate. Acked-by: Melissa Wen <mwen@igalia.com> Signed-off-by: Maíra Canal <mcanal@igalia.com>
The bcm2835_asb_control() function uses a tight polling loop to wait for the ASB bridge to acknowledge a request. During intensive workloads, this handshake intermittently fails for V3D's master ASB on BCM2711, resulting in "Failed to disable ASB master for v3d" errors during runtime PM suspend. As consequence, the failed power-off leaves V3D in a broken state, leading to bus faults or system hangs on later accesses. As the timeout is insufficient in some scenarios, increase the polling timeout from 1us to 5us, which is still negligible in the context of a power domain transition. Also, move the start timestamp to after the MMIO write, as the write latency is counted against the timeout, reducing the effective wait time for the hardware to respond. Signed-off-by: Maíra Canal <mcanal@igalia.com>
Simplify optional reset handling by using the function devm_reset_control_get_optional_exclusive(). Reviewed-by: Melissa Wen <mwen@igalia.com> Signed-off-by: Maíra Canal <mcanal@igalia.com>
Move all resource allocation operations before actually enabling the clock, as those operations don't require the GPU to be powered on. This is a preparation for runtime PM support. The next commit will move all code related to powering on and initiating the GPU into the runtime PM resume callback and all resource allocation will happen before resume(). Reviewed-by: Melissa Wen <mwen@igalia.com> Signed-off-by: Maíra Canal <mcanal@igalia.com>
Commit 90a64ad ("drm/v3d: Get rid of pm code") removed the last bits of power management code that V3D had, which were actually never hooked. Therefore, currently, the GPU clock is enabled during probe and only disabled when removing the driver. Implement proper power management using the kernel's Runtime PM framework. Signed-off-by: Maíra Canal <mcanal@igalia.com>
Contributor
Yeah, I ran several performance tests and also did power measurements with this branch last week and the results were good (no performance regressions + good power savings). The branch also survived Mesa CI. I hope this will be the last version of the series, so I'm okay with downstreaming it earlier. I'll make sure to open an PR in the case that changes happen before upstreaming. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently on rpi-6.18.y we never enter the idle voltage state due to v3d clock apparently remaining high.
This is due to kernel using the CLOCK_SET_STATE to indicate V3D clock is no longer used (with CLOCK_SET_RATE still indicating the high clock).
The firmware currently doesn't handle this correctly and wants an explicit CLOCK_SET_RATE.
While the firmware should support this at a future time, a kernel side fix would be a useful workaround for users without updated bootloader/firmware.
e.g. at idle state, we see:
After this PR we see the desired:
@mairacanal I've grabbed these commits from your v3d/downstream/power-management-v6 tree.
Does this seem reasonable for testing?
We are hoping to move to 6.18 kernel in near future and having some testing early would be useful.
I'm aware there may be changes during upstreaming - but we can switch to updated commits when available.