On 29/07/25 13:19, Maíra Canal wrote:
Hi Maxime,
On 29/07/25 09:14, Maxime Ripard wrote:
On Tue, Jul 29, 2025 at 08:53:51AM -0300, Maíra Canal wrote:
Hi Maxime,
On 29/07/25 04:27, Maxime Ripard wrote:
Hi Maíra,
On Mon, Jul 28, 2025 at 09:35:38AM -0300, Maíra Canal wrote:
Currently, when we prepare or unprepare RPi's clocks, we don't
actually
enable/disable the firmware clock. This means that
`clk_disable_unprepare()` doesn't actually change the clock state at
all, nor does it lowers the clock rate.
From the Mailbox Property Interface documentation [1], we can see
that
we should use `RPI_FIRMWARE_SET_CLOCK_STATE` to set the clock state
off/on. Therefore, use `RPI_FIRMWARE_SET_CLOCK_STATE` to create a
prepare and an unprepare hook for RPi's firmware clock.
As now the clocks are actually turned off, some of them are now marked
with CLK_IGNORE_UNUSED or CLK_IS_CRITICAL, as those are required since
early boot or are required during reboot.
What difference is there between the CLK_IGNORE_UNUSED and
CLK_IS_CRITICAL clocks?
From my understanding, CLK_IGNORE_UNUSED will prevent the clock to be
gated during boot (on "clk: Disabling unused clocks"), but after it, the
clock can be gated.
With CLK_IS_CRITICAL, the clock will never be disabled.
Yeah, that's correct.
For example, RPI_FIRMWARE_M2MC_CLK_ID is used by vc4. It needs to be
enabled at boot (I tested; if not enabled, it won't boot). However,
after vc4 is probed, we would like vc4 to have control of it and be able
to unprepare it in `vc4_hdmi_runtime_suspend()`. If I set it as
CLK_IS_CRITICAL, vc4 won't be able to unprepare it.
If the clock can be disabled by Linux, but it breaks some drivers if
it's not enabled during their probe, something is fishy somewhere, and
it's likely it would be just as broken if you compiled the driver as a
module.
Even then, some of the other clocks should probably never be disabled,
like the CPU clock.
I'll mark RPI_FIRMWARE_ARM_CLK_ID and RPI_FIRMWARE_CORE_CLK_ID as
critical. Are there any other clocks you think should never be disabled?
I only set RPI_FIRMWARE_PIXEL_BVB_CLK_ID as critical, as, otherwise, the
RPi won't reboot.
Why?
I'll have to dig a bit into vc4 HDMI code and to investigate the reason
(and maybe fix the issue there).
After some investigation, I believe that those display-related should be
set to CLK_IGNORE_UNUSED. It's not that it breaks some drivers if not
enabled, but it breaks hardware functionality and the device won't boot.
See, for example, clk-bcm2835 in which all PLL and PLL dividers clocks
are marked with CLK_IGNORE_UNUSED and some with CLK_IS_CRITICAL.
Maybe Dave has some input about the topic?
So far, I'm planing to keep CLK_IGNORE_UNUSED to the display-related
clocks and remove CLK_IS_CRITICAL from RPI_FIRMWARE_PIXEL_BVB_CLK_ID. If
you have any objections about it, let me know.
Best Regards,
- Maíra
Best Regards,
- Maíra
