2021-05-07 17:19:52

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 00/11] drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec

Hi,



hdmi-codec allows to have a lot of HDMI-audio related infrastructure in place,

it's missing a few controls to be able to provide HBR passthrough. This series

adds more infrastructure for the drivers, and leverages it in the vc4 HDMI

controller driver.



One thing that felt a bit weird is that even though

https://www.kernel.org/doc/html/latest/sound/kernel-api/writing-an-alsa-driver.html#iec958-s-pdif

mentions that the iec958 mask control should be a mixer control and the

default control should be a PCM one, it feels a bit weird to have two different

control type for two controls so similar, and other drivers are pretty

inconsistent with this. Should we update the documentation?



Thanks!

Maxime



Dom Cobley (5):

drm/vc4: hdmi: Set HD_CTL_WHOLSMP and HD_CTL_CHALIGN_SET

drm/vc4: hdmi: Set HDMI_MAI_FMT

drm/vc4: hdmi: Set VC4_HDMI_MAI_CONFIG_FORMAT_REVERSE

drm/vc4: hdmi: Remove firmware logic for MAI threshold setting

ARM: dts: bcm2711: Tune DMA parameters for HDMI audio



Maxime Ripard (6):

snd: iec958: split status creation and fill

ASoC: hdmi-codec: Rework to support more controls

ASoC: hdmi-codec: Add iec958 controls

ASoC: hdmi-codec: Add a prepare hook

drm/vc4: hdmi: Register HDMI codec

drm/vc4: hdmi: Remove redundant variables



arch/arm/boot/dts/bcm2711.dtsi | 4 +-

drivers/gpu/drm/vc4/Kconfig | 1 +

drivers/gpu/drm/vc4/vc4_hdmi.c | 322 ++++++++++++++-------------------

drivers/gpu/drm/vc4/vc4_hdmi.h | 5 +-

drivers/gpu/drm/vc4/vc4_regs.h | 30 +++

include/sound/hdmi-codec.h | 12 +-

include/sound/pcm_iec958.h | 8 +

sound/core/pcm_iec958.c | 131 +++++++++-----

sound/soc/codecs/hdmi-codec.c | 219 +++++++++++++++++-----

9 files changed, 456 insertions(+), 276 deletions(-)



--

2.31.1




2021-05-07 17:20:01

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 07/11] drm/vc4: hdmi: Set VC4_HDMI_MAI_CONFIG_FORMAT_REVERSE

From: Dom Cobley <[email protected]>

Without this bit set, HDMI_MAI_FORMAT doesn't pick up
the format and samplerate from DVP_CFG_MAI0_FMT and you
can't get HDMI_HDMI_13_AUDIO_STATUS_1 to indicate HBR mode

Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 9d33ac464a2d..f74a6b99d4ec 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1232,6 +1232,7 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,

HDMI_WRITE(HDMI_MAI_CONFIG,
VC4_HDMI_MAI_CONFIG_BIT_REVERSE |
+ VC4_HDMI_MAI_CONFIG_FORMAT_REVERSE |
VC4_SET_FIELD(channel_mask, VC4_HDMI_MAI_CHANNEL_MASK));

channel_map = vc4_hdmi->variant->channel_map(vc4_hdmi, channel_mask);
--
2.31.1

2021-05-07 17:20:17

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 02/11] ASoC: hdmi-codec: Rework to support more controls

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]>
---
sound/soc/codecs/hdmi-codec.c | 43 ++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 422539f933de..bc0d695d2df0 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -621,21 +621,23 @@ static const struct snd_soc_dai_ops hdmi_codec_spdif_dai_ops = {
SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE |\
SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE)

-static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
- struct snd_soc_dai *dai)
-{
- struct snd_soc_dai_driver *drv = dai->driver;
- struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
- struct snd_kcontrol *kctl;
- struct snd_kcontrol_new hdmi_eld_ctl = {
- .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+struct snd_kcontrol_new hdmi_codec_controls[] = {
+ {
+ .access = (SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE),
.iface = SNDRV_CTL_ELEM_IFACE_PCM,
.name = "ELD",
.info = hdmi_eld_ctl_info,
.get = hdmi_eld_ctl_get,
- .device = rtd->pcm->device,
- };
+ },
+};
+
+static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_dai_driver *drv = dai->driver;
+ struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+ unsigned int i;
int ret;

ret = snd_pcm_add_chmap_ctls(rtd->pcm, SNDRV_PCM_STREAM_PLAYBACK,
@@ -652,12 +654,21 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;

- /* add ELD ctl with the device number corresponding to the PCM stream */
- kctl = snd_ctl_new1(&hdmi_eld_ctl, dai->component);
- if (!kctl)
- return -ENOMEM;
+ for (i = 0; i < ARRAY_SIZE(hdmi_codec_controls); i++) {
+ struct snd_kcontrol *kctl;

- return snd_ctl_add(rtd->card->snd_card, kctl);
+ /* add ELD ctl with the device number corresponding to the PCM stream */
+ kctl = snd_ctl_new1(&hdmi_codec_controls[i], dai->component);
+ if (!kctl)
+ return -ENOMEM;
+
+ kctl->id.device = rtd->pcm->device;
+ ret = snd_ctl_add(rtd->card->snd_card, kctl);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
}

static int hdmi_dai_probe(struct snd_soc_dai *dai)
--
2.31.1

2021-05-07 17:20:35

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 03/11] ASoC: hdmi-codec: Add iec958 controls

Signed-off-by: Maxime Ripard <[email protected]>
---
sound/soc/codecs/hdmi-codec.c | 66 +++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index bc0d695d2df0..45081f928680 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -278,6 +278,7 @@ struct hdmi_codec_priv {
bool busy;
struct snd_soc_jack *jack;
unsigned int jack_status;
+ u8 iec_status[5];
};

static const struct snd_soc_dapm_widget hdmi_widgets[] = {
@@ -386,6 +387,47 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol *kcontrol,
return 0;
}

+static int hdmi_codec_iec958_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
+ uinfo->count = 1;
+ return 0;
+}
+
+static int hdmi_codec_iec958_default_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+ struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+
+ memcpy(ucontrol->value.iec958.status, hcp->iec_status,
+ sizeof(hcp->iec_status));
+
+ return 0;
+}
+
+static int hdmi_codec_iec958_default_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+ struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+
+ memcpy(hcp->iec_status, ucontrol->value.iec958.status,
+ sizeof(hcp->iec_status));
+
+ return 0;
+}
+
+static int hdmi_codec_iec958_mask_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ memset(ucontrol->value.iec958.status, 0xff,
+ sizeof_field(struct hdmi_codec_priv, iec_status));
+
+ return 0;
+}
+
static int hdmi_codec_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
@@ -460,8 +502,9 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
params_width(params), params_rate(params),
params_channels(params));

- ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status,
- sizeof(hp.iec.status));
+ memcpy(hp.iec.status, hcp->iec_status, sizeof(hp->iec_status));
+ ret = snd_pcm_fill_iec958_consumer_hw_params(params, hp.iec.status,
+ sizeof(hp.iec.status));
if (ret < 0) {
dev_err(dai->dev, "Creating IEC958 channel status failed %d\n",
ret);
@@ -622,6 +665,20 @@ static const struct snd_soc_dai_ops hdmi_codec_spdif_dai_ops = {
SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE)

struct snd_kcontrol_new hdmi_codec_controls[] = {
+ {
+ .access = SNDRV_CTL_ELEM_ACCESS_READ,
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, MASK),
+ .info = hdmi_codec_iec958_info,
+ .get = hdmi_codec_iec958_mask_get,
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
+ .info = hdmi_codec_iec958_info,
+ .get = hdmi_codec_iec958_default_get,
+ .put = hdmi_codec_iec958_default_put,
+ },
{
.access = (SNDRV_CTL_ELEM_ACCESS_READ |
SNDRV_CTL_ELEM_ACCESS_VOLATILE),
@@ -874,6 +931,11 @@ static int hdmi_codec_probe(struct platform_device *pdev)
hcp->hcd = *hcd;
mutex_init(&hcp->lock);

+ ret = snd_pcm_create_iec958_consumer_default(hcp->iec_status,
+ sizeof(hcp->iec_status));
+ if (ret < 0)
+ return ret;
+
daidrv = devm_kcalloc(dev, dai_count, sizeof(*daidrv), GFP_KERNEL);
if (!daidrv)
return -ENOMEM;
--
2.31.1

2021-05-07 17:20:46

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 08/11] drm/vc4: hdmi: Remove firmware logic for MAI threshold setting

From: Dom Cobley <[email protected]>

This was a workaround for bugs in hardware on earlier Pi models
and wasn't totally successful.

It makes audio quality worse on a Pi4 at the higher sample rates

Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index f74a6b99d4ec..505574e6cfd3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1213,22 +1213,12 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
audio_packet_config |= VC4_SET_FIELD(channel_mask,
VC4_HDMI_AUDIO_PACKET_CEA_MASK);

- /* Set the MAI threshold. This logic mimics the firmware's. */
- if (vc4_hdmi->audio.samplerate > 96000) {
- HDMI_WRITE(HDMI_MAI_THR,
- VC4_SET_FIELD(0x12, VC4_HD_MAI_THR_DREQHIGH) |
- VC4_SET_FIELD(0x12, VC4_HD_MAI_THR_DREQLOW));
- } else if (vc4_hdmi->audio.samplerate > 48000) {
- HDMI_WRITE(HDMI_MAI_THR,
- VC4_SET_FIELD(0x14, VC4_HD_MAI_THR_DREQHIGH) |
- VC4_SET_FIELD(0x12, VC4_HD_MAI_THR_DREQLOW));
- } else {
- HDMI_WRITE(HDMI_MAI_THR,
- VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICHIGH) |
- VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICLOW) |
- VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQHIGH) |
- VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQLOW));
- }
+ /* Set the MAI threshold */
+ HDMI_WRITE(HDMI_MAI_THR,
+ VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICHIGH) |
+ VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICLOW) |
+ VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQHIGH) |
+ VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQLOW));

HDMI_WRITE(HDMI_MAI_CONFIG,
VC4_HDMI_MAI_CONFIG_BIT_REVERSE |
--
2.31.1

2021-05-07 17:20:55

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 06/11] drm/vc4: hdmi: Set HDMI_MAI_FMT

From: Dom Cobley <[email protected]>

The hardware uses this for generating the right audio
data island packets when using formats other than PCM

Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 48 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/vc4/vc4_regs.h | 30 +++++++++++++++++++++
2 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 459d76414a29..9d33ac464a2d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1125,6 +1125,44 @@ static void vc4_hdmi_audio_shutdown(struct snd_pcm_substream *substream,
vc4_hdmi->audio.substream = NULL;
}

+static int sample_rate_to_mai_fmt(int samplerate)
+{
+ switch (samplerate) {
+ case 8000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_8000;
+ case 11025:
+ return VC4_HDMI_MAI_SAMPLE_RATE_11025;
+ case 12000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_12000;
+ case 16000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_16000;
+ case 22050:
+ return VC4_HDMI_MAI_SAMPLE_RATE_22050;
+ case 24000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_24000;
+ case 32000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_32000;
+ case 44100:
+ return VC4_HDMI_MAI_SAMPLE_RATE_44100;
+ case 48000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_48000;
+ case 64000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_64000;
+ case 88200:
+ return VC4_HDMI_MAI_SAMPLE_RATE_88200;
+ case 96000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_96000;
+ case 128000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_128000;
+ case 176400:
+ return VC4_HDMI_MAI_SAMPLE_RATE_176400;
+ case 192000:
+ return VC4_HDMI_MAI_SAMPLE_RATE_192000;
+ default:
+ return VC4_HDMI_MAI_SAMPLE_RATE_NOT_INDICATED;
+ }
+}
+
/* HDMI audio codec callbacks */
static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
@@ -1135,6 +1173,8 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
struct device *dev = &vc4_hdmi->pdev->dev;
u32 audio_packet_config, channel_mask;
u32 channel_map;
+ u32 mai_audio_format;
+ u32 mai_sample_rate;

if (substream != vc4_hdmi->audio.substream)
return -EINVAL;
@@ -1155,6 +1195,14 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,

vc4_hdmi_audio_set_mai_clock(vc4_hdmi);

+ mai_sample_rate = sample_rate_to_mai_fmt(vc4_hdmi->audio.samplerate);
+ mai_audio_format = VC4_HDMI_MAI_FORMAT_PCM;
+ HDMI_WRITE(HDMI_MAI_FMT,
+ VC4_SET_FIELD(mai_sample_rate,
+ VC4_HDMI_MAI_FORMAT_SAMPLE_RATE) |
+ VC4_SET_FIELD(mai_audio_format,
+ VC4_HDMI_MAI_FORMAT_AUDIO_FORMAT));
+
/* The B frame identifier should match the value used by alsa-lib (8) */
audio_packet_config =
VC4_HDMI_AUDIO_PACKET_ZERO_DATA_ON_SAMPLE_FLAT |
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index be2c32a519b3..489f921ef44d 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -516,6 +516,36 @@
# define VC4_HDMI_AUDIO_PACKET_CEA_MASK_MASK VC4_MASK(7, 0)
# define VC4_HDMI_AUDIO_PACKET_CEA_MASK_SHIFT 0

+# define VC4_HDMI_MAI_FORMAT_AUDIO_FORMAT_MASK VC4_MASK(23, 16)
+# define VC4_HDMI_MAI_FORMAT_AUDIO_FORMAT_SHIFT 16
+
+enum {
+ VC4_HDMI_MAI_FORMAT_PCM = 2,
+ VC4_HDMI_MAI_FORMAT_HBR = 200,
+};
+
+# define VC4_HDMI_MAI_FORMAT_SAMPLE_RATE_MASK VC4_MASK(15, 8)
+# define VC4_HDMI_MAI_FORMAT_SAMPLE_RATE_SHIFT 8
+
+enum {
+ VC4_HDMI_MAI_SAMPLE_RATE_NOT_INDICATED = 0,
+ VC4_HDMI_MAI_SAMPLE_RATE_8000 = 1,
+ VC4_HDMI_MAI_SAMPLE_RATE_11025 = 2,
+ VC4_HDMI_MAI_SAMPLE_RATE_12000 = 3,
+ VC4_HDMI_MAI_SAMPLE_RATE_16000 = 4,
+ VC4_HDMI_MAI_SAMPLE_RATE_22050 = 5,
+ VC4_HDMI_MAI_SAMPLE_RATE_24000 = 6,
+ VC4_HDMI_MAI_SAMPLE_RATE_32000 = 7,
+ VC4_HDMI_MAI_SAMPLE_RATE_44100 = 8,
+ VC4_HDMI_MAI_SAMPLE_RATE_48000 = 9,
+ VC4_HDMI_MAI_SAMPLE_RATE_64000 = 10,
+ VC4_HDMI_MAI_SAMPLE_RATE_88200 = 11,
+ VC4_HDMI_MAI_SAMPLE_RATE_96000 = 12,
+ VC4_HDMI_MAI_SAMPLE_RATE_128000 = 13,
+ VC4_HDMI_MAI_SAMPLE_RATE_176400 = 14,
+ VC4_HDMI_MAI_SAMPLE_RATE_192000 = 15,
+};
+
# define VC4_HDMI_RAM_PACKET_ENABLE BIT(16)

/* When set, the CTS_PERIOD counts based on MAI bus sync pulse instead
--
2.31.1

2021-05-07 17:21:05

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 09/11] drm/vc4: hdmi: Register HDMI codec

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]>
---
drivers/gpu/drm/vc4/Kconfig | 1 +
drivers/gpu/drm/vc4/vc4_hdmi.c | 243 +++++++++++----------------------
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +-
3 files changed, 80 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 118e8a426b1a..345a5570a3da 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -12,6 +12,7 @@ config DRM_VC4
select SND_PCM
select SND_PCM_ELD
select SND_SOC_GENERIC_DMAENGINE_PCM
+ select SND_SOC_HDMI_CODEC
select DRM_MIPI_DSI
help
Choose this option if you have a system that has a Broadcom
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 505574e6cfd3..19739b57d067 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -45,6 +45,7 @@
#include <linux/rational.h>
#include <linux/reset.h>
#include <sound/dmaengine_pcm.h>
+#include <sound/hdmi-codec.h>
#include <sound/pcm_drm_eld.h>
#include <sound/pcm_params.h>
#include <sound/soc.h>
@@ -420,15 +421,10 @@ static void vc4_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
static void vc4_hdmi_set_audio_infoframe(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct hdmi_audio_infoframe *audio = &vc4_hdmi->audio.infoframe;
union hdmi_infoframe frame;

- hdmi_audio_infoframe_init(&frame.audio);
-
- frame.audio.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM;
- frame.audio.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM;
- frame.audio.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM;
- frame.audio.channels = vc4_hdmi->audio.channels;
-
+ memcpy(&frame.audio, audio, sizeof(*audio));
vc4_hdmi_write_infoframe(encoder, &frame);
}

@@ -1063,18 +1059,10 @@ static inline struct vc4_hdmi *dai_to_hdmi(struct snd_soc_dai *dai)
return snd_soc_card_get_drvdata(card);
}

-static int vc4_hdmi_audio_startup(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
+static int vc4_hdmi_audio_startup(struct device *dev, void *data)
{
- struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+ struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
- struct drm_connector *connector = &vc4_hdmi->connector;
- int ret;
-
- if (vc4_hdmi->audio.substream && vc4_hdmi->audio.substream != substream)
- return -EINVAL;
-
- vc4_hdmi->audio.substream = substream;

/*
* If the HDMI encoder hasn't probed, or the encoder is
@@ -1084,15 +1072,18 @@ static int vc4_hdmi_audio_startup(struct snd_pcm_substream *substream,
VC4_HDMI_RAM_PACKET_ENABLE))
return -ENODEV;

- ret = snd_pcm_hw_constraint_eld(substream->runtime, connector->eld);
- if (ret)
- return ret;
+ vc4_hdmi->audio.streaming = true;

- return 0;
-}
+ HDMI_WRITE(HDMI_MAI_CTL,
+ VC4_HD_MAI_CTL_RESET |
+ VC4_HD_MAI_CTL_FLUSH |
+ VC4_HD_MAI_CTL_DLATE |
+ VC4_HD_MAI_CTL_ERRORE |
+ VC4_HD_MAI_CTL_ERRORF);
+
+ if (vc4_hdmi->variant->phy_rng_enable)
+ vc4_hdmi->variant->phy_rng_enable(vc4_hdmi);

-static int vc4_hdmi_audio_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
-{
return 0;
}

@@ -1112,17 +1103,20 @@ static void vc4_hdmi_audio_reset(struct vc4_hdmi *vc4_hdmi)
HDMI_WRITE(HDMI_MAI_CTL, VC4_HD_MAI_CTL_FLUSH);
}

-static void vc4_hdmi_audio_shutdown(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
+static void vc4_hdmi_audio_shutdown(struct device *dev, void *data)
{
- struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+ struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);

- if (substream != vc4_hdmi->audio.substream)
- return;
+ HDMI_WRITE(HDMI_MAI_CTL,
+ VC4_HD_MAI_CTL_DLATE |
+ VC4_HD_MAI_CTL_ERRORE |
+ VC4_HD_MAI_CTL_ERRORF);

+ if (vc4_hdmi->variant->phy_rng_disable)
+ vc4_hdmi->variant->phy_rng_disable(vc4_hdmi);
+
+ vc4_hdmi->audio.streaming = false;
vc4_hdmi_audio_reset(vc4_hdmi);
-
- vc4_hdmi->audio.substream = NULL;
}

static int sample_rate_to_mai_fmt(int samplerate)
@@ -1164,39 +1158,38 @@ static int sample_rate_to_mai_fmt(int samplerate)
}

/* HDMI audio codec callbacks */
-static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *params,
- struct snd_soc_dai *dai)
+static int vc4_hdmi_audio_prepare(struct device *dev, void *data,
+ struct hdmi_codec_daifmt *daifmt,
+ struct hdmi_codec_params *params)
{
- struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
+ struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
- struct device *dev = &vc4_hdmi->pdev->dev;
u32 audio_packet_config, channel_mask;
u32 channel_map;
u32 mai_audio_format;
u32 mai_sample_rate;

- if (substream != vc4_hdmi->audio.substream)
- return -EINVAL;
-
dev_dbg(dev, "%s: %u Hz, %d bit, %d channels\n", __func__,
- params_rate(params), params_width(params),
- params_channels(params));
+ params->sample_rate, params->sample_width,
+ params->channels);

- vc4_hdmi->audio.channels = params_channels(params);
- vc4_hdmi->audio.samplerate = params_rate(params);
+ vc4_hdmi->audio.channels = params->channels;
+ vc4_hdmi->audio.samplerate = params->sample_rate;

HDMI_WRITE(HDMI_MAI_CTL,
- VC4_HD_MAI_CTL_RESET |
- VC4_HD_MAI_CTL_FLUSH |
- VC4_HD_MAI_CTL_DLATE |
- VC4_HD_MAI_CTL_ERRORE |
- VC4_HD_MAI_CTL_ERRORF);
+ VC4_SET_FIELD(params->channels, VC4_HD_MAI_CTL_CHNUM) |
+ VC4_HD_MAI_CTL_WHOLSMP |
+ VC4_HD_MAI_CTL_CHALIGN |
+ VC4_HD_MAI_CTL_ENABLE);

vc4_hdmi_audio_set_mai_clock(vc4_hdmi);

mai_sample_rate = sample_rate_to_mai_fmt(vc4_hdmi->audio.samplerate);
- mai_audio_format = VC4_HDMI_MAI_FORMAT_PCM;
+ if (params->iec.status[0] & IEC958_AES0_NONAUDIO &&
+ params->channels == 8)
+ mai_audio_format = VC4_HDMI_MAI_FORMAT_HBR;
+ else
+ mai_audio_format = VC4_HDMI_MAI_FORMAT_PCM;
HDMI_WRITE(HDMI_MAI_FMT,
VC4_SET_FIELD(mai_sample_rate,
VC4_HDMI_MAI_FORMAT_SAMPLE_RATE) |
@@ -1230,94 +1223,12 @@ static int vc4_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
vc4_hdmi_set_n_cts(vc4_hdmi);

+ memcpy(&vc4_hdmi->audio.infoframe, &params->cea, sizeof(params->cea));
vc4_hdmi_set_audio_infoframe(encoder);

return 0;
}

-static int vc4_hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
- struct snd_soc_dai *dai)
-{
- struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai);
-
- switch (cmd) {
- case SNDRV_PCM_TRIGGER_START:
- vc4_hdmi->audio.streaming = true;
-
- if (vc4_hdmi->variant->phy_rng_enable)
- vc4_hdmi->variant->phy_rng_enable(vc4_hdmi);
-
- HDMI_WRITE(HDMI_MAI_CTL,
- VC4_SET_FIELD(vc4_hdmi->audio.channels,
- VC4_HD_MAI_CTL_CHNUM) |
- VC4_HD_MAI_CTL_WHOLSMP |
- VC4_HD_MAI_CTL_CHALIGN |
- VC4_HD_MAI_CTL_ENABLE);
- break;
- case SNDRV_PCM_TRIGGER_STOP:
- HDMI_WRITE(HDMI_MAI_CTL,
- VC4_HD_MAI_CTL_DLATE |
- VC4_HD_MAI_CTL_ERRORE |
- VC4_HD_MAI_CTL_ERRORF);
-
- if (vc4_hdmi->variant->phy_rng_disable)
- vc4_hdmi->variant->phy_rng_disable(vc4_hdmi);
-
- vc4_hdmi->audio.streaming = false;
-
- break;
- default:
- break;
- }
-
- return 0;
-}
-
-static inline struct vc4_hdmi *
-snd_component_to_hdmi(struct snd_soc_component *component)
-{
- struct snd_soc_card *card = snd_soc_component_get_drvdata(component);
-
- return snd_soc_card_get_drvdata(card);
-}
-
-static int vc4_hdmi_audio_eld_ctl_info(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_info *uinfo)
-{
- struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
- struct vc4_hdmi *vc4_hdmi = snd_component_to_hdmi(component);
- struct drm_connector *connector = &vc4_hdmi->connector;
-
- uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
- uinfo->count = sizeof(connector->eld);
-
- return 0;
-}
-
-static int vc4_hdmi_audio_eld_ctl_get(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
- struct vc4_hdmi *vc4_hdmi = snd_component_to_hdmi(component);
- struct drm_connector *connector = &vc4_hdmi->connector;
-
- memcpy(ucontrol->value.bytes.data, connector->eld,
- sizeof(connector->eld));
-
- return 0;
-}
-
-static const struct snd_kcontrol_new vc4_hdmi_audio_controls[] = {
- {
- .access = SNDRV_CTL_ELEM_ACCESS_READ |
- SNDRV_CTL_ELEM_ACCESS_VOLATILE,
- .iface = SNDRV_CTL_ELEM_IFACE_PCM,
- .name = "ELD",
- .info = vc4_hdmi_audio_eld_ctl_info,
- .get = vc4_hdmi_audio_eld_ctl_get,
- },
-};
-
static const struct snd_soc_dapm_widget vc4_hdmi_audio_widgets[] = {
SND_SOC_DAPM_OUTPUT("TX"),
};
@@ -1328,8 +1239,6 @@ static const struct snd_soc_dapm_route vc4_hdmi_audio_routes[] = {

static const struct snd_soc_component_driver vc4_hdmi_audio_component_drv = {
.name = "vc4-hdmi-codec-dai-component",
- .controls = vc4_hdmi_audio_controls,
- .num_controls = ARRAY_SIZE(vc4_hdmi_audio_controls),
.dapm_widgets = vc4_hdmi_audio_widgets,
.num_dapm_widgets = ARRAY_SIZE(vc4_hdmi_audio_widgets),
.dapm_routes = vc4_hdmi_audio_routes,
@@ -1340,28 +1249,6 @@ static const struct snd_soc_component_driver vc4_hdmi_audio_component_drv = {
.non_legacy_dai_naming = 1,
};

-static const struct snd_soc_dai_ops vc4_hdmi_audio_dai_ops = {
- .startup = vc4_hdmi_audio_startup,
- .shutdown = vc4_hdmi_audio_shutdown,
- .hw_params = vc4_hdmi_audio_hw_params,
- .set_fmt = vc4_hdmi_audio_set_fmt,
- .trigger = vc4_hdmi_audio_trigger,
-};
-
-static struct snd_soc_dai_driver vc4_hdmi_audio_codec_dai_drv = {
- .name = "vc4-hdmi-hifi",
- .playback = {
- .stream_name = "Playback",
- .channels_min = 2,
- .channels_max = 8,
- .rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
- SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
- SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 |
- SNDRV_PCM_RATE_192000,
- .formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE,
- },
-};
-
static const struct snd_soc_component_driver vc4_hdmi_audio_cpu_dai_comp = {
.name = "vc4-hdmi-cpu-dai-component",
};
@@ -1388,7 +1275,6 @@ static struct snd_soc_dai_driver vc4_hdmi_audio_cpu_dai_drv = {
SNDRV_PCM_RATE_192000,
.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE,
},
- .ops = &vc4_hdmi_audio_dai_ops,
};

static const struct snd_dmaengine_pcm_config pcm_conf = {
@@ -1396,6 +1282,31 @@ static const struct snd_dmaengine_pcm_config pcm_conf = {
.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
};

+
+static int vc4_hdmi_audio_get_eld(struct device *dev, void *data,
+ uint8_t *buf, size_t len)
+{
+ struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+ struct drm_connector *connector = &vc4_hdmi->connector;
+
+ memcpy(buf, connector->eld, min(sizeof(connector->eld), len));
+
+ return 0;
+}
+
+static const struct hdmi_codec_ops vc4_hdmi_codec_ops = {
+ .get_eld = vc4_hdmi_audio_get_eld,
+ .prepare = vc4_hdmi_audio_prepare,
+ .audio_shutdown = vc4_hdmi_audio_shutdown,
+ .audio_startup = vc4_hdmi_audio_startup,
+};
+
+struct hdmi_codec_pdata vc4_hdmi_codec_pdata = {
+ .ops = &vc4_hdmi_codec_ops,
+ .max_i2s_channels = 8,
+ .i2s = 1,
+};
+
static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
{
const struct vc4_hdmi_register *mai_data =
@@ -1403,6 +1314,7 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
struct snd_soc_dai_link *dai_link = &vc4_hdmi->audio.link;
struct snd_soc_card *card = &vc4_hdmi->audio.card;
struct device *dev = &vc4_hdmi->pdev->dev;
+ struct platform_device *codec_pdev;
const __be32 *addr;
int index;
int ret;
@@ -1449,12 +1361,13 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
return ret;
}

- /* register component and codec dai */
- ret = devm_snd_soc_register_component(dev, &vc4_hdmi_audio_component_drv,
- &vc4_hdmi_audio_codec_dai_drv, 1);
- if (ret) {
- dev_err(dev, "Could not register component: %d\n", ret);
- return ret;
+ codec_pdev = platform_device_register_data(dev, HDMI_CODEC_DRV_NAME,
+ PLATFORM_DEVID_AUTO,
+ &vc4_hdmi_codec_pdata,
+ sizeof(vc4_hdmi_codec_pdata));
+ if (IS_ERR(codec_pdev)) {
+ dev_err(dev, "Couldn't register the HDMI codec: %ld\n", PTR_ERR(codec_pdev));
+ return PTR_ERR(codec_pdev);
}

dai_link->cpus = &vc4_hdmi->audio.cpu;
@@ -1467,9 +1380,9 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)

dai_link->name = "MAI";
dai_link->stream_name = "MAI PCM";
- dai_link->codecs->dai_name = vc4_hdmi_audio_codec_dai_drv.name;
+ dai_link->codecs->dai_name = "i2s-hifi";
dai_link->cpus->dai_name = dev_name(dev);
- dai_link->codecs->name = dev_name(dev);
+ dai_link->codecs->name = dev_name(&codec_pdev->dev);
dai_link->platforms->name = dev_name(dev);

card->dai_link = dai_link;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..fdee27faafa3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -111,8 +111,7 @@ struct vc4_hdmi_audio {
int samplerate;
int channels;
struct snd_dmaengine_dai_dma_data dma_data;
- struct snd_pcm_substream *substream;
-
+ struct hdmi_audio_infoframe infoframe;
bool streaming;
};

--
2.31.1

2021-05-07 17:21:13

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 11/11] ARM: dts: bcm2711: Tune DMA parameters for HDMI audio

From: Dom Cobley <[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]>
---
arch/arm/boot/dts/bcm2711.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 462b1dfb0385..8a7350cfcd9c 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -352,7 +352,7 @@ hdmi0: hdmi@7ef00700 {
interrupt-names = "cec-tx", "cec-rx", "cec-low",
"wakeup", "hpd-connected", "hpd-removed";
ddc = <&ddc0>;
- dmas = <&dma 10>;
+ dmas = <&dma (10 | (1 << 27) | (1 << 24)| (15 << 20) | (10 << 16))>;
dma-names = "audio-rx";
status = "disabled";
};
@@ -395,7 +395,7 @@ hdmi1: hdmi@7ef05700 {
<9>, <10>, <11>;
interrupt-names = "cec-tx", "cec-rx", "cec-low",
"wakeup", "hpd-connected", "hpd-removed";
- dmas = <&dma 17>;
+ dmas = <&dma (17 | (1 << 27) | (1 << 24)| (15 << 20) | (10 << 16))>;
dma-names = "audio-rx";
status = "disabled";
};
--
2.31.1

2021-05-07 17:52:03

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 04/11] ASoC: hdmi-codec: Add a prepare hook

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]>
---
include/sound/hdmi-codec.h | 12 +++-
sound/soc/codecs/hdmi-codec.c | 112 ++++++++++++++++++++++++++--------
2 files changed, 99 insertions(+), 25 deletions(-)

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 4b3a1d374b90..4fc733c8c570 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -65,12 +65,22 @@ struct hdmi_codec_ops {

/*
* Configures HDMI-encoder for audio stream.
- * Mandatory
+ * Having either prepare or hw_params is mandatory.
*/
int (*hw_params)(struct device *dev, void *data,
struct hdmi_codec_daifmt *fmt,
struct hdmi_codec_params *hparms);

+ /*
+ * Configures HDMI-encoder for audio stream. Can be called
+ * multiple times for each setup.
+ *
+ * Having either prepare or hw_params is mandatory.
+ */
+ int (*prepare)(struct device *dev, void *data,
+ struct hdmi_codec_daifmt *fmt,
+ struct hdmi_codec_params *hparms);
+
/*
* Shuts down the audio stream.
* Mandatory
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 45081f928680..08b5880e8ca4 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -482,6 +482,42 @@ static void hdmi_codec_shutdown(struct snd_pcm_substream *substream,
mutex_unlock(&hcp->lock);
}

+static int hdmi_codec_fill_codec_params(struct snd_soc_dai *dai,
+ unsigned int sample_width,
+ unsigned int sample_rate,
+ unsigned int channels,
+ struct hdmi_codec_params *hp)
+{
+ struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+ int idx;
+
+ /* Select a channel allocation that matches with ELD and pcm channels */
+ idx = hdmi_codec_get_ch_alloc_table_idx(hcp, channels);
+ if (idx < 0) {
+ dev_err(dai->dev, "Not able to map channels to speakers (%d)\n",
+ idx);
+ hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
+ return idx;
+ }
+
+ memset(hp, 0, sizeof(*hp));
+
+ hdmi_audio_infoframe_init(&hp->cea);
+ hp->cea.channels = channels;
+ hp->cea.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM;
+ hp->cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM;
+ hp->cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM;
+ hp->cea.channel_allocation = hdmi_codec_channel_alloc[idx].ca_id;
+
+ hp->sample_width = sample_width;
+ hp->sample_rate = sample_rate;
+ hp->channels = channels;
+
+ hcp->chmap_idx = hdmi_codec_channel_alloc[idx].ca_id;
+
+ return 0;
+}
+
static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -496,13 +532,24 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
.dig_subframe = { 0 },
}
};
- int ret, idx;
+ int ret;
+
+ if (!hcp->hcd.ops->hw_params)
+ return 0;

dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
params_width(params), params_rate(params),
params_channels(params));

- memcpy(hp.iec.status, hcp->iec_status, sizeof(hp->iec_status));
+ ret = hdmi_codec_fill_codec_params(dai,
+ params_width(params),
+ params_rate(params),
+ params_channels(params),
+ &hp);
+ if (ret < 0)
+ return ret;
+
+ memcpy(hp.iec.status, hcp->iec_status, sizeof(hp.iec.status));
ret = snd_pcm_fill_iec958_consumer_hw_params(params, hp.iec.status,
sizeof(hp.iec.status));
if (ret < 0) {
@@ -511,32 +558,47 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
return ret;
}

- hdmi_audio_infoframe_init(&hp.cea);
- hp.cea.channels = params_channels(params);
- hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM;
- hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM;
- hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM;
-
- /* Select a channel allocation that matches with ELD and pcm channels */
- idx = hdmi_codec_get_ch_alloc_table_idx(hcp, hp.cea.channels);
- if (idx < 0) {
- dev_err(dai->dev, "Not able to map channels to speakers (%d)\n",
- idx);
- hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
- return idx;
- }
- hp.cea.channel_allocation = hdmi_codec_channel_alloc[idx].ca_id;
- hcp->chmap_idx = hdmi_codec_channel_alloc[idx].ca_id;
-
- hp.sample_width = params_width(params);
- hp.sample_rate = params_rate(params);
- hp.channels = params_channels(params);
-
cf->bit_fmt = params_format(params);
return hcp->hcd.ops->hw_params(dai->dev->parent, hcp->hcd.data,
cf, &hp);
}

+static int hdmi_codec_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+ struct hdmi_codec_daifmt *cf = dai->playback_dma_data;
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ unsigned int channels = runtime->channels;
+ unsigned int width = snd_pcm_format_width(runtime->format);
+ unsigned int rate = runtime->rate;
+ struct hdmi_codec_params hp;
+ int ret;
+
+ if (!hcp->hcd.ops->prepare)
+ return 0;
+
+ dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
+ width, rate, channels);
+
+ ret = hdmi_codec_fill_codec_params(dai, width, rate, channels, &hp);
+ if (ret < 0)
+ return ret;
+
+ memcpy(hp.iec.status, hcp->iec_status, sizeof(hp.iec.status));
+ ret = snd_pcm_fill_iec958_consumer(runtime, hp.iec.status,
+ sizeof(hp.iec.status));
+ if (ret < 0) {
+ dev_err(dai->dev, "Creating IEC958 channel status failed %d\n",
+ ret);
+ return ret;
+ }
+
+ cf->bit_fmt = runtime->format;
+ return hcp->hcd.ops->prepare(dai->dev->parent, hcp->hcd.data,
+ cf, &hp);
+}
+
static int hdmi_codec_i2s_set_fmt(struct snd_soc_dai *dai,
unsigned int fmt)
{
@@ -628,6 +690,7 @@ static const struct snd_soc_dai_ops hdmi_codec_i2s_dai_ops = {
.startup = hdmi_codec_startup,
.shutdown = hdmi_codec_shutdown,
.hw_params = hdmi_codec_hw_params,
+ .prepare = hdmi_codec_prepare,
.set_fmt = hdmi_codec_i2s_set_fmt,
.mute_stream = hdmi_codec_mute,
};
@@ -918,7 +981,8 @@ static int hdmi_codec_probe(struct platform_device *pdev)
}

dai_count = hcd->i2s + hcd->spdif;
- if (dai_count < 1 || !hcd->ops || !hcd->ops->hw_params ||
+ if (dai_count < 1 || !hcd->ops ||
+ (!hcd->ops->hw_params && !hcd->ops->prepare) ||
!hcd->ops->audio_shutdown) {
dev_err(dev, "%s: Invalid parameters\n", __func__);
return -EINVAL;
--
2.31.1

2021-05-07 17:52:17

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 10/11] drm/vc4: hdmi: Remove redundant variables

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++++++++++++--------------
drivers/gpu/drm/vc4/vc4_hdmi.h | 2 --
2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 19739b57d067..a5780da70c1c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1008,12 +1008,13 @@ static u32 vc5_hdmi_channel_map(struct vc4_hdmi *vc4_hdmi, u32 channel_mask)
}

/* HDMI audio codec callbacks */
-static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *vc4_hdmi)
+static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *vc4_hdmi,
+ unsigned int samplerate)
{
u32 hsm_clock = clk_get_rate(vc4_hdmi->audio_clock);
unsigned long n, m;

- rational_best_approximation(hsm_clock, vc4_hdmi->audio.samplerate,
+ rational_best_approximation(hsm_clock, samplerate,
VC4_HD_MAI_SMP_N_MASK >>
VC4_HD_MAI_SMP_N_SHIFT,
(VC4_HD_MAI_SMP_M_MASK >>
@@ -1025,12 +1026,11 @@ static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *vc4_hdmi)
VC4_SET_FIELD(m - 1, VC4_HD_MAI_SMP_M));
}

-static void vc4_hdmi_set_n_cts(struct vc4_hdmi *vc4_hdmi)
+static void vc4_hdmi_set_n_cts(struct vc4_hdmi *vc4_hdmi, unsigned int samplerate)
{
struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
struct drm_crtc *crtc = encoder->crtc;
const struct drm_display_mode *mode = &crtc->state->adjusted_mode;
- u32 samplerate = vc4_hdmi->audio.samplerate;
u32 n, cts;
u64 tmp;

@@ -1164,27 +1164,25 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data,
{
struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
struct drm_encoder *encoder = &vc4_hdmi->encoder.base.base;
+ unsigned int sample_rate = params->sample_rate;
+ unsigned int channels = params->channels;
u32 audio_packet_config, channel_mask;
u32 channel_map;
u32 mai_audio_format;
u32 mai_sample_rate;

dev_dbg(dev, "%s: %u Hz, %d bit, %d channels\n", __func__,
- params->sample_rate, params->sample_width,
- params->channels);
-
- vc4_hdmi->audio.channels = params->channels;
- vc4_hdmi->audio.samplerate = params->sample_rate;
+ sample_rate, params->sample_width, channels);

HDMI_WRITE(HDMI_MAI_CTL,
- VC4_SET_FIELD(params->channels, VC4_HD_MAI_CTL_CHNUM) |
+ VC4_SET_FIELD(channels, VC4_HD_MAI_CTL_CHNUM) |
VC4_HD_MAI_CTL_WHOLSMP |
VC4_HD_MAI_CTL_CHALIGN |
VC4_HD_MAI_CTL_ENABLE);

- vc4_hdmi_audio_set_mai_clock(vc4_hdmi);
+ vc4_hdmi_audio_set_mai_clock(vc4_hdmi, sample_rate);

- mai_sample_rate = sample_rate_to_mai_fmt(vc4_hdmi->audio.samplerate);
+ mai_sample_rate = sample_rate_to_mai_fmt(sample_rate);
if (params->iec.status[0] & IEC958_AES0_NONAUDIO &&
params->channels == 8)
mai_audio_format = VC4_HDMI_MAI_FORMAT_HBR;
@@ -1202,7 +1200,7 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data,
VC4_HDMI_AUDIO_PACKET_ZERO_DATA_ON_INACTIVE_CHANNELS |
VC4_SET_FIELD(0x8, VC4_HDMI_AUDIO_PACKET_B_FRAME_IDENTIFIER);

- channel_mask = GENMASK(vc4_hdmi->audio.channels - 1, 0);
+ channel_mask = GENMASK(channels - 1, 0);
audio_packet_config |= VC4_SET_FIELD(channel_mask,
VC4_HDMI_AUDIO_PACKET_CEA_MASK);

@@ -1221,7 +1219,7 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data,
channel_map = vc4_hdmi->variant->channel_map(vc4_hdmi, channel_mask);
HDMI_WRITE(HDMI_MAI_CHANNEL_MAP, channel_map);
HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config);
- vc4_hdmi_set_n_cts(vc4_hdmi);
+ vc4_hdmi_set_n_cts(vc4_hdmi, sample_rate);

memcpy(&vc4_hdmi->audio.infoframe, &params->cea, sizeof(params->cea));
vc4_hdmi_set_audio_infoframe(encoder);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index fdee27faafa3..574f379faf93 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -108,8 +108,6 @@ struct vc4_hdmi_audio {
struct snd_soc_dai_link_component cpu;
struct snd_soc_dai_link_component codec;
struct snd_soc_dai_link_component platform;
- int samplerate;
- int channels;
struct snd_dmaengine_dai_dma_data dma_data;
struct hdmi_audio_infoframe infoframe;
bool streaming;
--
2.31.1

2021-05-24 13:40:55

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 00/11] drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec

Hi,

On Fri, May 07, 2021 at 04:03:23PM +0200, Maxime Ripard wrote:
> Hi,
>
> hdmi-codec allows to have a lot of HDMI-audio related infrastructure in place,
> it's missing a few controls to be able to provide HBR passthrough. This series
> adds more infrastructure for the drivers, and leverages it in the vc4 HDMI
> controller driver.
>
> One thing that felt a bit weird is that even though
> https://www.kernel.org/doc/html/latest/sound/kernel-api/writing-an-alsa-driver.html#iec958-s-pdif
> mentions that the iec958 mask control should be a mixer control and the
> default control should be a PCM one, it feels a bit weird to have two different
> control type for two controls so similar, and other drivers are pretty
> inconsistent with this. Should we update the documentation?

Any comments on this series?
Thanks!
Maxime


Attachments:
(No filename) (850.00 B)
signature.asc (235.00 B)
Download all attachments

2021-05-25 08:55:57

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 00/11] drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec

On Mon, 24 May 2021 15:39:04 +0200,
Maxime Ripard wrote:
>
> Hi,
>
> On Fri, May 07, 2021 at 04:03:23PM +0200, Maxime Ripard wrote:
> > Hi,
> >
> > hdmi-codec allows to have a lot of HDMI-audio related infrastructure in place,
> > it's missing a few controls to be able to provide HBR passthrough. This series
> > adds more infrastructure for the drivers, and leverages it in the vc4 HDMI
> > controller driver.
> >
> > One thing that felt a bit weird is that even though
> > https://www.kernel.org/doc/html/latest/sound/kernel-api/writing-an-alsa-driver.html#iec958-s-pdif
> > mentions that the iec958 mask control should be a mixer control and the
> > default control should be a PCM one, it feels a bit weird to have two different
> > control type for two controls so similar, and other drivers are pretty
> > inconsistent with this. Should we update the documentation?
>
> Any comments on this series?

A patch for updating the documentation is welcome.
Currently, as de facto standard, we allow both MIXER and PCM ifaces
for all IEC958-related controls, and it's unlikely that we would
change that in future.


thanks,

Takashi

2021-05-25 09:25:32

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 00/11] drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec

Hi Takashi,

On Tue, May 25, 2021 at 10:35:14AM +0200, Takashi Iwai wrote:
> On Mon, 24 May 2021 15:39:04 +0200,
> Maxime Ripard wrote:
> >
> > Hi,
> >
> > On Fri, May 07, 2021 at 04:03:23PM +0200, Maxime Ripard wrote:
> > > Hi,
> > >
> > > hdmi-codec allows to have a lot of HDMI-audio related infrastructure in place,
> > > it's missing a few controls to be able to provide HBR passthrough. This series
> > > adds more infrastructure for the drivers, and leverages it in the vc4 HDMI
> > > controller driver.
> > >
> > > One thing that felt a bit weird is that even though
> > > https://www.kernel.org/doc/html/latest/sound/kernel-api/writing-an-alsa-driver.html#iec958-s-pdif
> > > mentions that the iec958 mask control should be a mixer control and the
> > > default control should be a PCM one, it feels a bit weird to have two different
> > > control type for two controls so similar, and other drivers are pretty
> > > inconsistent with this. Should we update the documentation?
> >
> > Any comments on this series?
>
> A patch for updating the documentation is welcome.
> Currently, as de facto standard, we allow both MIXER and PCM ifaces
> for all IEC958-related controls, and it's unlikely that we would
> change that in future.

Ok, I'll write a patch for the documentation make it clearer then :)

Do we want to make sure that all the iec958 controls are on the same
iface, or is it also left to the driver (or should we just leave the
existing drivers as is but encourage a consistent use in the future)?

Maxime


Attachments:
(No filename) (1.53 kB)
signature.asc (235.00 B)
Download all attachments

2021-05-25 10:17:26

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 00/11] drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec

On Tue, 25 May 2021 11:23:53 +0200,
Maxime Ripard wrote:
>
> Hi Takashi,
>
> On Tue, May 25, 2021 at 10:35:14AM +0200, Takashi Iwai wrote:
> > On Mon, 24 May 2021 15:39:04 +0200,
> > Maxime Ripard wrote:
> > >
> > > Hi,
> > >
> > > On Fri, May 07, 2021 at 04:03:23PM +0200, Maxime Ripard wrote:
> > > > Hi,
> > > >
> > > > hdmi-codec allows to have a lot of HDMI-audio related infrastructure in place,
> > > > it's missing a few controls to be able to provide HBR passthrough. This series
> > > > adds more infrastructure for the drivers, and leverages it in the vc4 HDMI
> > > > controller driver.
> > > >
> > > > One thing that felt a bit weird is that even though
> > > > https://www.kernel.org/doc/html/latest/sound/kernel-api/writing-an-alsa-driver.html#iec958-s-pdif
> > > > mentions that the iec958 mask control should be a mixer control and the
> > > > default control should be a PCM one, it feels a bit weird to have two different
> > > > control type for two controls so similar, and other drivers are pretty
> > > > inconsistent with this. Should we update the documentation?
> > >
> > > Any comments on this series?
> >
> > A patch for updating the documentation is welcome.
> > Currently, as de facto standard, we allow both MIXER and PCM ifaces
> > for all IEC958-related controls, and it's unlikely that we would
> > change that in future.
>
> Ok, I'll write a patch for the documentation make it clearer then :)
>
> Do we want to make sure that all the iec958 controls are on the same
> iface, or is it also left to the driver (or should we just leave the
> existing drivers as is but encourage a consistent use in the future)?

I'd leave the existing drivers as-is. Changing the iface is basically
an incompatible change, and although most of applications and alsa-lib
should look at both ifaces, there can be any surprise by that change.


thanks,

Takashi