-
Notifications
You must be signed in to change notification settings - Fork 34
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
HDMI CEC wakeup support #197
Conversation
Peripherals like DW HDMI controllers support only 8-bit access. Add 8-bit read and write helpers. Note that due to byte reordering on bus, 2 LSBs must be inverted in order to access correct address. Signed-off-by: Jernej Skrabec <[email protected]>
Note that CEC on Linux for some reason doesn't work after resume. I'll look into that. However, all important registers are preserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
I am okay with depending on Linux to initialize the hardware, as long as the driver is disabled by default, and that dependency is noted in the Kconfig help text.
Before merging, I would like to resolve (or at least understand) the issue preventing CEC from working after resume.
Actually, apart from clocks and resets (in CCU and additional ones in controller), there is nothing to initialize. Since I'm running this with debug uart and IR enabled, I only worry that in deeper sleep modes this won't work. Bus must still be responsive, which means 24 MHz clock must be running.
Sure, me too. |
Code should be much more readable now with magic numbers moved to macros. Probably not all wake up messages should be enabled. For one or two doesn't really make sense IMO. Linux issue was on my side. I removed one patch, which I thought it would be needed, but it was not. CEC now works correctly after resume. All in all, this works pretty well, except when debug uart is disabled (no wake up source works, not even the button). I guess driver should make sure that 24 MHz clock is never stopped. Any ideas how to correctly do that? I speculate that H6 has some changes in this department to allow deeper sleep modes and it might need different handling than A64, H3 and H5. |
I suppose defining HDMI_DDC_CLK_GATING clock (offset 0x154 in A64 CCU) and assigning it 24 MHz clock as parent would be enough. |
🎉
All of the buses are reparented to OSC32K, so MMIO register access should generally work after OSC24M is stopped.
Hmm, this is a bit surprising. Since you mention you have an Orange Pi Win: you can select
You can limit the suspend depth by taking a reference to
This would more accurately describe the hardware, but it would create other issues, as Crust would gate the clock when it gets released, thus breaking Linux. So I suggest using
Yes, H6 has a separate HDMI_CEC_CLK that runs off OSC32K. So once the driver has a clock consumer, taking advantage of that should be easy. |
Documentation that I have states that isfrclk clock must be between 18 MHz and 27 MHz. While it's not described anywhere how AW integrated this core, I suspect that isfrclk means input for special function registers clock and it would be logical that it correlates with HDMI_DDC_CLK_GATING. There is another possible thing that's missing in DT. DW HDMI controller binding mentions another optional clock - CEC, which is not specified in DTs for 40 nm SoCs. Documentation states it must be connected to 32768 Hz source. This is of course 32k clock from RTC. I found that when searching for cause why CEC doesn't work on some H3 boards and does on others. It correlates with board having external 32k crystal or not. I also forced internal RC oscillator on board with external crystal and CEC stopped working soon after. Also adjusting RC oscillator by hand also caused CEC to work again. Anyway, that's good enough reason to update all those SoC DTSI files with additional CEC clock, connected to 32k clock.
If you check Linux H6 CCU driver, you'll notice that I forced CEC clock to PLL periph0 x2, because 32k didn't work (BSP Linux driver does the same). Additionally, boards like Tanix TX6 don't have external 32k crystal at all and 32k crystal on Beelink GS1 doesn't work always, so it's disabled.
Noted. |
This code adds support for DW HDMI CEC driver, which can wake up system once appropriate message is received from TV or other HDMI CEC compatible device. It's assumed that clocks and resets are already initialized by rich OS. Signed-off-by: Jernej Skrabec <[email protected]>
Claiming and releasing CLK_OSC24M works. So this code seems good for A64. Testing on other SoCs, especially on H6 is still needed. Probably next week... |
I tested this on Tanix TX6 (H6) and it works too. |
@jernejsk Good to hear! Do you think this is ready to merge? I can fix up the CI failure (minor style checks) when applying. |
Yeah, I think automatic mode is as good as it can get, so this PR is ready to be merged. There are downsides like waking up even if some other device was addressed, but such issues can only be solved by manual mode. I'll work on it after I get a board for sniffing HDMI signals. BTW, do you think is possible to continuously doing RC oscillator calibration in standby, like every 5 min or so? This would allow using CEC even on boards without external 32k crystal. CEC standard allows for about 5% deviation but RC oscillator is often worse. |
Purpose
This PR implements DW HDMI CEC driver, which can be used as a wakeup source. It assumes that clocks and resets are already initialized by rich OS.
Note that it's very important that CEC has stable 32 kHz clock source. On A64, H3 and H5 this means external 32 kHz crystal (RC oscillator is too unstable). On H6, CEC peripheral is usually clocked from pll-periph0-2x with fixed predivider.
Considerations for reviewers
It was tested on A64 only for now. It should work on H3 and H5 too, since they have same DW HDMI controller and PHY. There is no reason why it shouldn't work on H6 too, but due to a newer controller and different PHY it might not work correctly yet.
Closes #196