Fix H616/H618 HDMI audio clocking#9916
Conversation
joakimtoe1
requested review from
AGM1968,
JohnTheCoolingFan,
PanderMusubi,
TheSnowfield,
Tonymac32,
chraac,
igorpecovnik,
janprunk,
juanesf,
krachlatte,
pyavitz,
schwar3kat,
sgjava and
teknoid
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPatches expose H616 audio PLL clocks to DT, remove pll-audio-hs fixed post-divider, update AHUB DAM DTS to use the PLL clocks and add a disabled machine node, and adjust AHUB drivers to program module rate/divider and properly acquire/release clk_pll_audio_4x (applies to 6.18 and 7.0). ChangesH616 Audio PLL and AHUB Clock Fixes
Sequence Diagram(s)sequenceDiagram
participant DeviceTree
participant AHUB_Driver
participant CLK_PLL_AUDIO_4X
participant CLK_MODULE
participant BCLK_Register
DeviceTree->>AHUB_Driver: provide clk-pll-audio-hs & clk-pll-audio-4x phandles
AHUB_Driver->>CLK_PLL_AUDIO_4X: clk_get(clk_pll_audio_4x)
AHUB_Driver->>CLK_MODULE: clk_set_parent(clk_module, clk_pll_audio_4x)
AHUB_Driver->>CLK_MODULE: clk_set_rate(clk_module, freq_out)
AHUB_Driver->>CLK_MODULE: clk_enable(clk_module)
AHUB_Driver->>BCLK_Register: write BCLK divider = bclk_ratio
Note over AHUB_Driver: on remove or error
AHUB_Driver->>CLK_PLL_AUDIO_4X: clk_put(clk_pll_audio_4x)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/large
Hey @joakimtoe1! 👋Thanks for submitting your first pull request to the Armbian project — we're excited to have you contributing! 🧡 If you'd like to stay informed about project updates or collaborate more closely with the team, Also, don’t forget to ⭐ star the repo if you haven’t already — and welcome aboard! 🚀 |
github-actions
Bot
added
Needs review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
patch/kernel/archive/sunxi-7.0/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patch (1)
99-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame resource leak as the 6.18 patch:
clk_put(clk_info->clk_pll)is still commented out.Apply the same fix here to uncomment line 110.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/sunxi-7.0/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patch` around lines 99 - 112, The cleanup path is leaking clk_pll because the clk_put(clk_info->clk_pll) call is still commented out; in the error unwind labels (err_module_clk_enable / err_pll_clk_enable / err_set_parent_clk / err_pllx4_clk) restore the missing release by uncommenting the clk_put(clk_info->clk_pll) call under the err_pllx4_clk / err_pll_clk labels so that clk_put(clk_info->clk_pll) is invoked during error unwind before calling clk_put(clk_info->clk_module); keep the existing label order (err_set_parent_clk → err_pllx4_clk → err_pll_clk) and ensure no duplicate frees are introduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@patch/kernel/archive/sunxi-6.18/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patch`:
- Around line 99-112: The cleanup path for err_pllx4_clk currently doesn't
release clk_info->clk_pll, causing a resource leak; update the
err_pllx4_clk/err_set_parent_clk cleanup sequence to call
clk_put(clk_info->clk_pll) when err_pllx4_clk is reached (i.e., ensure
clk_put(clk_info->clk_pll) is executed before proceeding to release clk_module),
so that when clk_pllx4 acquisition fails the previously acquired clk_pll is
properly put.
---
Duplicate comments:
In
`@patch/kernel/archive/sunxi-7.0/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patch`:
- Around line 99-112: The cleanup path is leaking clk_pll because the
clk_put(clk_info->clk_pll) call is still commented out; in the error unwind
labels (err_module_clk_enable / err_pll_clk_enable / err_set_parent_clk /
err_pllx4_clk) restore the missing release by uncommenting the
clk_put(clk_info->clk_pll) call under the err_pllx4_clk / err_pll_clk labels so
that clk_put(clk_info->clk_pll) is invoked during error unwind before calling
clk_put(clk_info->clk_module); keep the existing label order (err_set_parent_clk
→ err_pllx4_clk → err_pll_clk) and ensure no duplicate frees are introduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2bd32f8c-d2a1-4351-99ac-5ff0cb9be17d
📒 Files selected for processing (4)
patch/kernel/archive/sunxi-6.18/patches.armbian/0700-clk-sunxi-ng-h616-fix-audio-pll-for-hdmi.patchpatch/kernel/archive/sunxi-6.18/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patchpatch/kernel/archive/sunxi-7.0/patches.armbian/0700-clk-sunxi-ng-h616-fix-audio-pll-for-hdmi.patchpatch/kernel/archive/sunxi-7.0/patches.armbian/0703-sunxi-h616-fix-ahub-hdmi-audio-clock-path.patch
|
I wonder how this was tested when the patches were not even applied (lacking entry in series.conf) |
joakimtoe1
force-pushed
the
fix-sunxi-h616-hdmi-audio
branch
from
d20c811 to
026b92f
Compare
You were right; the first version added standalone patch files but did not wire them into series.conf, so a clean patch-stack application would not have picked them up. I reworked the PR so the HDMI audio fix is folded into the existing 0702-arm64-dts-sun50i-h616-add-digital-audio-node.patch, which is already listed in both sunxi-6.18/series.conf and sunxi-7.0/series.conf. The branch has also been cleaned up to a single commit and rebuilt from that exact commit; the patch stage passes and the generated image/dtb packages were installed and tested on Orange Pi Zero 2W / H618 with audible HDMI audio confirmed. |
joakimtoe1
force-pushed
the
fix-sunxi-h616-hdmi-audio
branch
from
026b92f to
746da82
Compare
joakimtoe1
force-pushed
the
fix-sunxi-h616-hdmi-audio
branch
from
746da82 to
4c350f4
Compare
Fix HDMI audio on H616/H618 boards using the Armbian sunxi64 current and edge kernel patch stacks. On Orange Pi Zero 2W / H618, HDMI video worked and the HDMI ALSA card was created correctly. ELD was valid, ASoC routing was active, DW-HDMI I2S setup was reached, and PCM playback entered RUNNING, but no audible sound was produced by the TV. The failure needs both AHUB clock path correction and AHUB BCLK divider correction. A BCLK-only test programmed SUNXI_AHUB_I2S_CLKD(1) to the working 0x90 value, but left audio-hub at 43 MHz with pll-audio-hs/pll-audio-4x at 688 MHz and remained silent. This change folds the HDMI audio fix into the already-applied H616 digital audio patch for sunxi-6.18 and sunxi-7.0: - expose CLK_PLL_AUDIO_HS and CLK_PLL_AUDIO_4X to DT - use those real audio PLL clocks for AHUB DAM instead of audio-codec clocks - remove the fixed post divider from pll-audio-hs so exact SDM audio rates are used - parent the AHUB module clock to pll-audio-4x - set the AHUB module clock to the full requested rate instead of freq_out / 2 - program the AHUB BCLK divider field with the raw divider map value instead of bclk_ratio - 2 - make the HDMI controller node a DAI provider with #sound-dai-cells = <0>, so the local hdmi-audio-fix overlay is no longer needed Validated on Orange Pi Zero 2W / H618 with 48 kHz S16_LE and S32_LE HDMI speaker-test playback. The working runtime state has pll-audio-hs, pll-audio-4x, and audio-hub at 98.304 MHz, with SUNXI_AHUB_I2S_CLKD(1) programmed to 0x90. The final test was run without the hdmi-audio-fix overlay; HDMI still registered and ELD remained valid. Signed-off-by: joakimtoe <joakimtoe@gmail.com>
joakimtoe1
force-pushed
the
fix-sunxi-h616-hdmi-audio
branch
from
4c350f4 to
3452d03
Compare
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
✅ This PR has been reviewed and approved — all set for merge! |
3 participants
Description
Fix HDMI audio on H616/H618 boards using the Armbian sunxi64 current and edge kernel patch stacks.
On Orange Pi Zero 2W / H618, HDMI video worked and the HDMI ALSA card was created correctly. ELD was valid, ASoC routing was active, DW-HDMI
I2S setup was reached, and PCM playback entered
RUNNING, but no audible sound was produced by the TV.This PR updates the existing H616 digital audio patch for both
sunxi-6.18andsunxi-7.0to:CLK_PLL_AUDIO_HSandCLK_PLL_AUDIO_4Xpll-audio-hsso exact SDM audio rates are usedpll-audio-4xfreq_out / 2bclk_ratio - 2#sound-dai-cells = <0>, so a separate localhdmi-audio-fixoverlay is no longer neededGitHub issue reference: none
Jira reference number: none
Documentation summary for feature / change
No main documentation entry is expected. This is a kernel patch-stack fix for HDMI audio behavior on affected H616/H618 boards.
How Has This Been Tested?
Tested on Orange Pi Zero 2W / H618 with Armbian sunxi64 Linux 6.18.33.
RUNNING, no audible soundhdmi-audio-fixoverlay:user_overlays=speaker-test -D hw:1,0 -c 2 -r 48000 -F S32_LE -t sinerunsspeaker-test -D hw:1,0 -c 2 -r 48000 -F S16_LE -t sinerunsRUNNING0x0509730c = 0x00000090pll-audio-hs = 98.304 MHzpll-audio-4x = 98.304 MHzaudio-hub = 98.304 MHzAlso compared runtime AHUB/I2S/DW-HDMI registers against a working Batocera H616/H618 reference board. The relevant playback clock/register
state matched after this fix.
Checklist:
Summary by CodeRabbit