Skip to content

Improvements to hdmi-codec and conversion of vc4-hdmi #4311

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

Merged
merged 8 commits into from
Jun 16, 2021

Conversation

mripard
Copy link
Contributor

@mripard mripard commented Apr 28, 2021

Hi,

Here's a rework of the HBR passthrough and channel mapping support for vc4-hdmi to rely on the hdmi-codec driver instead of duplicating some of its code.

I've checked that the reported ELD is still valid (using amixer -c0 cget numid=$ID), and iecset to dump the iec status.

I don't have the hardware to test this (I think?) so it would be great if someone could test and make sure nothing got broken in the process. I'm especially worried about the ordering of the calls that is mentioned here: 06a9ff0

I couldn't find in Kodi, LibreElec or alsa-lib where the status setup was done and the order relative to the hw_params call so it might be broken at the moment.

Thanks!
Maxime

@popcornmix
Copy link
Collaborator

I've just given this a test and TrueHD passthrough worked but DTS-HD (despite making DTS-HD led on AVR come on) was silent.
Possibly related to the iec_status bits, but @HiassofT knows more about this.

@popcornmix
Copy link
Collaborator

popcornmix commented Apr 28, 2021

I'm wondering about the AES bits set here: https://github.com/xbmc/xbmc/blob/0e7f7628cb5fe8453fedf801b12cc659d2eb7bcb/xbmc/cores/AudioEngine/Sinks/AESinkALSA.cpp#L453
I think they get passed to driver and affect whether we are in PCM or passthrough mode.

(getting them wrong tends to be implementation defined - some sinks will make a best effort go at making a sound, others will just go silent is anything seems wrong).

@HiassofT
Copy link
Contributor

We can't select between PCM and HBR frames in hw_params, as the alsa hook plugin passes on the IEC status bits after hw_params (and before prepare) - therefore I moved the code to prepare.

See also the commit message of my change fd45fc3

@mripard mripard force-pushed the rpi/5.10-hdmi-codec branch 2 times, most recently from 5e629da to 86d7e05 Compare April 30, 2021 12:57
@mripard
Copy link
Contributor Author

mripard commented Apr 30, 2021

I've just pushed a new version that implements prepare

siajeeheng and others added 8 commits May 7, 2021 10:36
Existing hdmi-codec driver only support standard pcm format.
Support of IEC958 encoded format pass from ALSA IEC958 plugin is needed
so that the IEC958 encoded data can be streamed to the HDMI chip.

Signed-off-by: Sia Jee Heng <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
We're going to add more controls to support the IEC958 output, so let's
rework the control registration a bit to support more of them.

Signed-off-by: Maxime Ripard <[email protected]>
In some situations, like a codec probe, we need to provide an IEC status
default but don't have access to the sampling rate and width yet since
no stream has been configured yet.

Each and every driver has its own default, whereas the core iec958 code
also has some buried in the snd_pcm_create_iec958_consumer functions.

Let's split these functions in two to provide a default that doesn't
rely on the sampling rate and width, and another function to fill them
when available.

Signed-off-by: Maxime Ripard <[email protected]>
The IEC958 status bit is usually set by the userspace after hw_params
has been called, so in order to use whatever is set by the userspace, we
need to implement the prepare hook. Let's add it to the hdmi_codec_ops,
and mandate that either prepare or hw_params is implemented.

Signed-off-by: Maxime Ripard <[email protected]>
The hdmi-codec brings a lot of advanced features, including the HDMI
channel mapping. Let's use it in our driver instead of our own codec.

Signed-off-by: Maxime Ripard <[email protected]>
Enable NO_WAIT_RESP, DMA_WIDE_SOURCE, DMA_WIDE_DEST, and bump the DMA
panic and AXI priorities to avoid any DMA transfer error with HBR audio
(8 channel, 192Hz).

Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
@mripard mripard force-pushed the rpi/5.10-hdmi-codec branch from 86d7e05 to 7c08add Compare May 7, 2021 09:13
@mripard
Copy link
Contributor Author

mripard commented May 7, 2021

I just pushed a new version addressing the comments from @jernejsk and @HiassofT

@popcornmix
Copy link
Collaborator

Just tested update PR and TrueHD passthrough is still silent for me. Removing this PR and audio is back.

@HiassofT
Copy link
Contributor

HiassofT commented May 7, 2021

@popcornmix did you update vc4-hdmi.conf? HiassofT/LibreELEC.tv@bbde777

@popcornmix
Copy link
Collaborator

No - let me try that.

@popcornmix
Copy link
Collaborator

Yes, that seems to be working for a range of passthrough formats and multichannel PCM.
Looks like a nice clean up.

@HiassofT I assume it was fine for you?

@HiassofT
Copy link
Contributor

HiassofT commented May 7, 2021

Yes, it's fine for me.

Haven't tested the latest version (and can't test multichannel PCM output / channel mapping) but I glanced through the changes and they looked OK to me.

Might be worth sending out an RFC series and see what upstream ASoC/dri folks think about the hdmi-codec changes

@mripard
Copy link
Contributor Author

mripard commented May 7, 2021

It's prepared already, I was waiting for your testing results to send it. If it works as expected for you, I'll send it in the afternoon

@mripard
Copy link
Contributor Author

mripard commented May 7, 2021

@mripard
Copy link
Contributor Author

mripard commented Jun 16, 2021

The ASoC parts have been accepted upstream and are on their way to 5.13

@popcornmix
Copy link
Collaborator

Does this PR still match upstream version? (not sure if it's been reworked since this PR's version).

@mripard
Copy link
Contributor Author

mripard commented Jun 16, 2021

The commits are not identical in the ASoC part but it's only about the commit logs and some additional documentation I wrote in order to upstream. The code itself is identical.

The DRM side is fairly different but it's mostly due to fact that we had to revert the old code in the downstream tree, while we only had to add new code upstream.

@popcornmix
Copy link
Collaborator

@pelwell I think this should be merged. I've been running with it for a while at home without issue and it's merged upstream.

@pelwell pelwell merged commit 70de7e6 into raspberrypi:rpi-5.10.y Jun 16, 2021
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jun 16, 2021
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Jun 16, 2021
@mripard mripard deleted the rpi/5.10-hdmi-codec branch June 17, 2021 08:53
@pelwell
Copy link
Contributor

pelwell commented Jun 17, 2021

Some of these don't apply cleanly to rpi-5.12.y (I've merged the ones that do).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants