[v7,0/3] Add HDMI audio on the rk3588 SoC

Message

Detlev Casanova Feb. 17, 2025, 9:47 p.m. UTC

To support HDMI audio on the rk3588 based devices, the generic HDMI
Codec framework is used in the dw-hdmi-qp DRM bridge driver.

The implementation is mainly based on the downstream driver, ported to the
generic HDMI Codec framework [1] recently merged in the master branch.
The parameters computation has been kept as is and the data stored in the
dw_hdmi_qp struct as been cleaned up.

The table for the N values has been edited to reflect N recommended values
as well as CTS recommended values.

The downstream kernel also implements a machine driver for HDMI audio but
it is doing exactly what the simple-audio-card driver does, so use that
instead in the RK3588 SoC device tree.

This adds HDMI audio support for both HDMI TX ports.

*** Dependencies ***

Based on Linus' master branch, but also needs Cristian's dts patches for HDMI1
support [2], which depends on Heiko's patchset for
phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI will
not work (at all).

[1]: https://lore.kernel.org/all/[e-mail]
[2]: https://lore.kernel.org/linux-rockchip/[e-mail]
[3]: https://lore.kernel.org/lkml/[e-mail]/

Changes since v6:
 - Fix arguments alignement (checkpatch --strict warnings)
 - Add hdmi1 audio support too
 - Move hdmi0_sound node under hdmi0_out_con

Changes since v5:
 - Simplify audio math computation for N
 - Move hdmi0-sound node up with other address-less nodes

Changes since v4:
 - Moved hdmi0_audio node the rk3588-base.dtsi
 - Enable hdmi0_audio in rk3588-rock-5b.dts

Changes since v3:
 - Renamed function to start with dw_hdmi_qp

Changes since v2:
 - Also clear the audio infoframe
 - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
 - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
 - Clear tmds_char_rate in atomic_disable and only write registers when
   tmds_char_rate is not 0.
 - Do not use connector_state duplicates

Changes since v1:
 - Remove useless audio_mutex (was used downstream for multiple drivers access
   to audio functions)
 - Let hdmi_codec build and setup audio infoframes
 - Only access audio registers when connector is connected
 - Rebased on master branch

Detlev Casanova (2):
  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B

Sugar Zhang (1):
  drm/bridge: synopsys: Add audio support for dw-hdmi-qp

 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
 .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
 .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
 4 files changed, 539 insertions(+)

Comments

Piotr Oniszczuk Feb. 20, 2025, 11:16 a.m. UTC | #1

> Wiadomość napisana przez Detlev Casanova <[e-mail]> w dniu 17 lut 2025, o godz. 22:47:
> 
> To support HDMI audio on the rk3588 based devices, the generic HDMI
> Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> 
> The implementation is mainly based on the downstream driver, ported to the
> generic HDMI Codec framework [1] recently merged in the master branch.
> The parameters computation has been kept as is and the data stored in the
> dw_hdmi_qp struct as been cleaned up.
> 
> The table for the N values has been edited to reflect N recommended values
> as well as CTS recommended values.
> 
> The downstream kernel also implements a machine driver for HDMI audio but
> it is doing exactly what the simple-audio-card driver does, so use that
> instead in the RK3588 SoC device tree.
> 
> This adds HDMI audio support for both HDMI TX ports.
> 
> *** Dependencies ***
> 
> Based on Linus' master branch, but also needs Cristian's dts patches for HDMI1
> support [2], which depends on Heiko's patchset for
> phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI will
> not work (at all).
> 
> [1]: https://lore.kernel.org/all/[e-mail]
> [2]: https://lore.kernel.org/linux-rockchip/[e-mail]
> [3]: https://lore.kernel.org/lkml/[e-mail]/
> 
> Changes since v6:
> - Fix arguments alignement (checkpatch --strict warnings)
> - Add hdmi1 audio support too
> - Move hdmi0_sound node under hdmi0_out_con
> 
> Changes since v5:
> - Simplify audio math computation for N
> - Move hdmi0-sound node up with other address-less nodes
> 
> Changes since v4:
> - Moved hdmi0_audio node the rk3588-base.dtsi
> - Enable hdmi0_audio in rk3588-rock-5b.dts
> 
> Changes since v3:
> - Renamed function to start with dw_hdmi_qp
> 
> Changes since v2:
> - Also clear the audio infoframe
> - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
> - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
> - Clear tmds_char_rate in atomic_disable and only write registers when
>   tmds_char_rate is not 0.
> - Do not use connector_state duplicates
> 
> Changes since v1:
> - Remove useless audio_mutex (was used downstream for multiple drivers access
>   to audio functions)
> - Let hdmi_codec build and setup audio infoframes
> - Only access audio registers when connector is connected
> - Rebased on master branch
> 
> Detlev Casanova (2):
>  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
>  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B
> 
> Sugar Zhang (1):
>  drm/bridge: synopsys: Add audio support for dw-hdmi-qp
> 
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
> .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
> .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
> 4 files changed, 539 insertions(+)
> 
> -- 
> 2.48.1
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


Detelv,

Just curious of your opinion:

I’m on 6.14-rc3 and using https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge + this v7 series.

On mine rock5b all works nicely but - at boot time - i’m getting some oops in kernel like this: https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722

I’m not sure where issue is but it looks to me like kind of interference between analog audio and hdmi audio as commenting analog audio in dts ( https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts?h=v6.14-rc3#n24 ) stops kernel from oops’ing….

 rock5b dts i’m using is like this: https://gist.github.com/warpme/a8a32afd4a05d4b7f25d2808984b05ac

Detlev Casanova Feb. 20, 2025, 5:03 p.m. UTC | #2

Hi Piotr,

On Thursday, 20 February 2025 06:16:20 EST Piotr Oniszczuk wrote:
> > Wiadomość napisana przez Detlev Casanova <[e-mail]> w
> > dniu 17 lut 2025, o godz. 22:47:
> > 
> > To support HDMI audio on the rk3588 based devices, the generic HDMI
> > Codec framework is used in the dw-hdmi-qp DRM bridge driver.
> > 
> > The implementation is mainly based on the downstream driver, ported to the
> > generic HDMI Codec framework [1] recently merged in the master branch.
> > The parameters computation has been kept as is and the data stored in the
> > dw_hdmi_qp struct as been cleaned up.
> > 
> > The table for the N values has been edited to reflect N recommended values
> > as well as CTS recommended values.
> > 
> > The downstream kernel also implements a machine driver for HDMI audio but
> > it is doing exactly what the simple-audio-card driver does, so use that
> > instead in the RK3588 SoC device tree.
> > 
> > This adds HDMI audio support for both HDMI TX ports.
> > 
> > *** Dependencies ***
> > 
> > Based on Linus' master branch, but also needs Cristian's dts patches for
> > HDMI1 support [2], which depends on Heiko's patchset for
> > phy-rockchip-samsung-hdptx [3]. Patches will apply without [3], but HDMI
> > will not work (at all).
> > 
> > [1]:
> > https://lore.kernel.org/all/20241224-drm-bridge-hdmi-connector-v10-0-dc89
> > [e-mail] [2]:
> > https://lore.kernel.org/linux-rockchip/20241211-rk3588-hdmi1-v2-0-02cdca2
> > [e-mail] [3]:
> > https://lore.kernel.org/lkml/[e-mail]/
> > 
> > Changes since v6:
> > - Fix arguments alignement (checkpatch --strict warnings)
> > - Add hdmi1 audio support too
> > - Move hdmi0_sound node under hdmi0_out_con
> > 
> > Changes since v5:
> > - Simplify audio math computation for N
> > - Move hdmi0-sound node up with other address-less nodes
> > 
> > Changes since v4:
> > - Moved hdmi0_audio node the rk3588-base.dtsi
> > - Enable hdmi0_audio in rk3588-rock-5b.dts
> > 
> > Changes since v3:
> > - Renamed function to start with dw_hdmi_qp
> > 
> > Changes since v2:
> > - Also clear the audio infoframe
> > - Write AUDI_CONTENTS0 to its default value in case it gets overwritten.
> > - Store tmds_char_rate in the dw_hdmi_qp struct in atomic_enable
> > - Clear tmds_char_rate in atomic_disable and only write registers when
> > 
> >   tmds_char_rate is not 0.
> > 
> > - Do not use connector_state duplicates
> > 
> > Changes since v1:
> > - Remove useless audio_mutex (was used downstream for multiple drivers
> > access> 
> >   to audio functions)
> > 
> > - Let hdmi_codec build and setup audio infoframes
> > - Only access audio registers when connector is connected
> > - Rebased on master branch
> > 
> > Detlev Casanova (2):
> >  arm64: dts: rockchip: Add HDMI audio outputs for rk3588 SoC
> >  arm64: dts: rockchip: Enable HDMI audio outputs for Rock 5B
> > 
> > Sugar Zhang (1):
> >  drm/bridge: synopsys: Add audio support for dw-hdmi-qp
> > 
> > arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  17 +
> > .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  17 +
> > .../boot/dts/rockchip/rk3588-rock-5b.dts      |  16 +
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c  | 489 ++++++++++++++++++
> > 4 files changed, 539 insertions(+)
> 
> Detelv,
> 
> Just curious of your opinion:
> 
> I’m on 6.14-rc3 and using
> https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-brid
> ge + this v7 series.

Do you have the branch available somewhere ?

> On mine rock5b all works nicely but - at boot time - i’m getting some oops
> in kernel like this:
> https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722

I did notice that at one point but it disappeard after a rebase on the the 
latest master so I didn't look further into that.
Could it be related to 2518a0e1b878042f9afa45ae063e544a16efc1a3 "ASoC: simple-
card: use __free(device_node) for device node" ?

I'm not exactly sure how __free(device_node) works though, but reverting that 
commit could fix the issue.

> I’m not sure where issue is but it looks to me like kind of interference
> between analog audio and hdmi audio as commenting analog audio in dts (
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/
> arm64/boot/dts/rockchip/rk3588-rock-5b.dts?h=v6.14-rc3#n24 ) stops kernel
> from oops’ing….

I cannot reproduce anymore, so if you have a branch/config that you use, I 
could try, looking into that.
I don't see any relevant commits that would change this behavior on master 
since v6.14-rc3, so I'm not sure what is going on.

>  rock5b dts i’m using is like this:
> https://gist.github.com/warpme/a8a32afd4a05d4b7f25d2808984b05ac

Regards,
Detlev.

Piotr Oniszczuk Feb. 20, 2025, 6:31 p.m. UTC | #3

> Wiadomość napisana przez Detlev Casanova <[e-mail]> w dniu 20 lut 2025, o godz. 18:03:
> 
> Hi Piotr,
> 
> On Thursday, 20 February 2025 06:16:20 EST Piotr Oniszczuk wrote:
>> 
>> 
>> Detelv,
>> 
>> Just curious of your opinion:
>> 
>> I’m on 6.14-rc3 and using
>> https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-brid
>> ge + this v7 series.
> 
> Do you have the branch available somewhere ?

My tests are on my MiniMyth2 distro. 
My build system is vanilla upstream+patches style.  
Kernel is mainline 6.14-rc3 kernel with applied series of patches:

PATCHFILES += 1001-math.h-add-DIV_ROUND_UP_NO_OVERFLOW.patch
PATCHFILES += 1002-clk-divider-Fix-divisor-masking-on-64-bit-platforms.patch
PATCHFILES += 1003-clk-composite-replace-open-coded-abs_diff.patch
# hdmi video support
PATCHFILES += 1010-FROM-ML-phy-phy-rockchip-samsung-hdptx-Don-t-use-dt-.patch
PATCHFILES += 1011-FROM-UPSTREAM-drm-rockchip-Don-t-change-hdmi-referen.patch
PATCHFILES += 1012-FROM-UPSTREAM-drm-rockchip-vop2-Drop-unnecessary-if_.patch
PATCHFILES += 1013-FROM-UPSTREAM-drm-rockchip-vop2-Improve-display-mode.patch
PATCHFILES += 1014-WIP-drm-rockchip-vop2-Improve-display-modes-handling.patch
PATCHFILES += 1015-drm-bridge-dw-hdmi-Sync-comments-with-actual-bus-for.patch
PATCHFILES += 1016-drm-bridge-connector-Sync-supported_formats-with-com.patch
PATCHFILES += 1017-drm-connector-hdmi-Evaluate-limited-range-after-comp.patch
PATCHFILES += 1018-drm-connector-hdmi-Add-support-for-YUV420-format-ver.patch
PATCHFILES += 1019-drm-connector-hdmi-Improve-debug-message-for-support.patch
PATCHFILES += 1020-drm-connector-hdmi-Use-YUV420-output-format-as-an-RG.patch
PATCHFILES += 1021-phy-Add-HDMI-configuration-options.patch
PATCHFILES += 1022-phy-hdmi-Add-color-depth-configuration.patch
PATCHFILES += 1023-phy-rockchip-samsung-hdptx-Fix-clock-ratio-setup.patch
PATCHFILES += 1024-phy-rockchip-samsung-hdptx-Drop-unused-lcpll_config.patch
PATCHFILES += 1025-phy-rockchip-samsung-hdptx-Setup-TMDS-char-rate-via-.patch
PATCHFILES += 1026-phy-rockchip-samsung-hdptx-Add-high-color-depth-mana.patch
PATCHFILES += 1027-phy-rockchip-samsung-hdptx-Cleanup-internal-rate-han.patch
PATCHFILES += 1028-phy-rockchip-samsung-hdptx-Avoid-Hz-hHz-unit-convers.patch
PATCHFILES += 1029-TEST-phy-rockchip-samsung-hdptx-Add-verbose-logging.patch
PATCHFILES += 1030-WIP-drm-bridge-Add-detect_ctx-hook.patch
PATCHFILES += 1031-WIP-drm-bridge-connector-Switch-from-detect-to-detec.patch
PATCHFILES += 1032-WIP-drm-bridge-dw-hdmi-qp-Add-high-TMDS-clock-ratio-.patch
PATCHFILES += 1033-WIP-drm-rockchip-vop2-Add-high-color-depth-support.patch
PATCHFILES += 1034-WIP-drm-rockchip-vop2-Add-YUV420-support.patch
PATCHFILES += 1035-WIP-drm-rockchip-dw_hdmi_qp-Make-use-of-phy_configur.patch
PATCHFILES += 1036-WIP-drm-rockchip-dw_hdmi_qp-Add-10bpc-and-YUV420-out.patch
PATCHFILES += 1037-WIP-drm-bridge-dw-hdmi-qp-Enable-10bpc-and-YUV420.patch
# hdmi audio support
PATCHFILES += 1040-drm-bridge-synopsys-add-audio-support-for-dw-hdmi-qp-v7.patch
# cec support
PATCHFILES += 1045-drm-bridge-synopsys-add-cec-support.patch
# var additions
PATCHFILES += 1060-net-ethernet-add-yt6801-gige-pcie-controller.patch
PATCHFILES += 1061-net-ethernet-yt6801-gige-pcie-silence-debug-msgs.patch
PATCHFILES += 1062-WIP-iommu-rockchip-add-flush_iotlb_all-ops.patch
PATCHFILES += 1063-media-rockchip-add-rkvdec2-driver.patch
PATCHFILES += 1064-media-rkvdec2-add-iommu-support-v3.patch
PATCHFILES += 1065-wip-add-hevc-support.patch
PATCHFILES += 1066-wip-hevc-add-ref-frames-support.patch
# dtsi additions
PATCHFILES += 1070-arm64-dtsi-rk3588s-add-vop2-clock-resets.patch
PATCHFILES += 1071-arm64-dtsi-rockchip-3588s-add-hdmi-bridge.patch
PATCHFILES += 1072-arm64-dtsi-rockchip-3588-hdmi-add-audio-support.patch
PATCHFILES += 1074-arm64-dtsi-rockchip-add-rkvdec2-video-vecoder-on-rk3588.patch
PATCHFILES += 1077-arm64-dtsi-rkvdec2-add-iommu-support-v3.patch
PATCHFILES += 1078-arm64-dtsi-rockchip-rk356x-add-rkvdec2-video-decoder-nodes.patch
# dts patches
PATCHFILES += 1080-arm64-dts-rockchip-rk3588s-rock5a-dts-improvements.patch
PATCHFILES += 1081-arm64-dts-rockchip-rk3588-rock5b-dts-improvements.patch
PATCHFILES += 1082-arm64-dts-rockchip-rk3588s-rock5c-dts-improvements.patch
PATCHFILES += 1083-arm64-dts-rockchip-rk3588-rock5itx-dts-improvements.patch
PATCHFILES += 1084-arm64-dts-rockchip-rk3588s-opi5-dts-improvements.patch
PATCHFILES += 1085-arm64-dts-rockchip-rk3588-opi5plus-dts-improvements.patch
PATCHFILES += 1086-arm64-dts-rockchip-rk3588s-add-opi5pro-dts.patch
PATCHFILES += 1087-arm64-dts-rockchip-rk3588s-add-nanopi-m6-dts.patch
PATCHFILES += 1088-arm64-dts-rockchip-rk3588s-nanopc-r6s-dts-improvements.patch
PATCHFILES += 1089-arm64-dts-rockchip-rk3588-nanopc-t6-dtsi-improvements.patch
PATCHFILES += 1090-arm64-dts-rockchip-rk3588-add-rock5t-dt.patch

 patches are from: https://github.com/warpme/minimyth2/tree/master/script/kernel/linux-6.14/files

Kernel config is: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.14/files/linux-6.14-arm64-armv8.config

Above patching is effectively: 

- https://gitlab.collabora.com/cristicc/linux-next/-/commits/rk3588-hdmi-bridge
- Yours hdmi audio v7
. added cec support
- added rkvdec2 (including hevc)
- some dts enablements for above


> 
>> On mine rock5b all works nicely but - at boot time - i’m getting some oops
>> in kernel like this:
>> https://gist.github.com/warpme/e1668acbef7817e5d2a88a6840328722
> 
> I did notice that at one point but it disappeard after a rebase on the the 
> latest master so I didn't look further into that.

Indeed - i.e. i don’t have these oops on rk3588 based orange5plus.
Also 6.12 kernel is clean.
But i have them reproducible on rock5b (and also e.g. on rock5t)
  
> Could it be related to 2518a0e1b878042f9afa45ae063e544a16efc1a3 "ASoC: simple-
> card: use __free(device_node) for device node" ?
> 

I tried with 2518a0e1b878042f9afa45ae063e544a16efc1a3 revered and this NOT helps.
With reverted above commit, dmesg is: https://gist.github.com/warpme/dbfe813583e4660a02b74315f193e768

Piotr Oniszczuk Feb. 21, 2025, 11:42 a.m. UTC | #4

Small data point: on rock5b switching in dts analog audio: from audio-graph-card to simple-audio-card (so dts is: https://gist.github.com/warpme/349b27e49bc6f617ef1041047e75adab ) makes kernel oops go away with analog audio still working…

so maybe issue is in audio-graph-card code (or its dts fragments)?

Detlev Casanova Feb. 21, 2025, 2:43 p.m. UTC | #5

On Friday, 21 February 2025 06:42:31 EST Piotr Oniszczuk wrote:
> Small data point: on rock5b switching in dts analog audio: from
> audio-graph-card to simple-audio-card (so dts is:
> https://gist.github.com/warpme/349b27e49bc6f617ef1041047e75adab ) makes
> kernel oops go away with analog audio still working…
> 
> so maybe issue is in audio-graph-card code (or its dts fragments)?

I can reproduce with your config (I guess that removing the driver for analog 
audio hides the issue)

I'm really feel like simple_util_clean_reference(card) in simple_probe() 
errors path should not be called anymore, since
https://lore.kernel.org/all/[e-mail]/

I'm adding Kuninori Morimoto in the to list of this thread for extra input 
(See thread at https://lore.kernel.org/all/[e-mail]/) as they made that change.
Especially those commits:
      ASoC: audio-graph-card2: use __free(device_node) for device node
      ASoC: audio-graph-card: use __free(device_node) for device node
      ASoC: simple-card: use __free(device_node) for device node

Detlev.