[PATCH 0/2] Add support for sun4i HDMI audio

[Stefan Mavrodiev]

This patch series add support for HDMI audio for sun4i HDMI encored.
The code uses some parts from the Allwinners's BSP kernel.

Currently cyclic DMA transfers are disabled. The first patch permits them
as they are required for the audio.

The patch is tested on A20 chip. For the other chips, only the addresses
of the registers are checked.

Stefan Mavrodiev (2):
dmaengine: sun4i: Add support for cyclic requests with dedicated DMA
drm: sun4i: hdmi: Add support for sun4i HDMI encoder audio

drivers/dma/sun4i-dma.c | 45 +--
drivers/gpu/drm/sun4i/Kconfig | 1 +
drivers/gpu/drm/sun4i/Makefile | 1 +
drivers/gpu/drm/sun4i/sun4i_hdmi.h | 30 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 375 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 +
6 files changed, 435 insertions(+), 21 deletions(-)
create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c

--
2.17.1

[Stefan Mavrodiev]

Currently the cyclic transfers can be used only with normal DMAs. They
can be used by pcm_dmaengine module, which is required for implementing
sound with sun4i-hdmi encoder. This is so because the controller can
accept audio only from a dedicated DMA.

This patch enables them, following the existing style for the
scatter/gather type transfers.

Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---
drivers/dma/sun4i-dma.c | 45 ++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index e397a50058c8..7b41815d86fb 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -669,43 +669,41 @@ sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t len,
dma_addr_t src, dest;
u32 endpoints;
int nr_periods, offset, plength, i;
+ u8 ram_type, io_mode, linear_mode;

if (!is_slave_direction(dir)) {
dev_err(chan2dev(chan), "Invalid DMA direction\n");
return NULL;
}

- if (vchan->is_dedicated) {
- /*
- * As we are using this just for audio data, we need to use
- * normal DMA. There is nothing stopping us from supporting
- * dedicated DMA here as well, so if a client comes up and
- * requires it, it will be simple to implement it.
- */
- dev_err(chan2dev(chan),
- "Cyclic transfers are only supported on Normal DMA\n");
- return NULL;
- }
-
contract = generate_dma_contract();
if (!contract)
return NULL;

contract->is_cyclic = 1;

- /* Figure out the endpoints and the address we need */
+ if (vchan->is_dedicated) {
+ io_mode = SUN4I_DDMA_ADDR_MODE_IO;
+ linear_mode = SUN4I_DDMA_ADDR_MODE_LINEAR;
+ ram_type = SUN4I_DDMA_DRQ_TYPE_SDRAM;
+ } else {
+ io_mode = SUN4I_NDMA_ADDR_MODE_IO;
+ linear_mode = SUN4I_NDMA_ADDR_MODE_LINEAR;
+ ram_type = SUN4I_NDMA_DRQ_TYPE_SDRAM;
+ }
+
if (dir == DMA_MEM_TO_DEV) {
src = buf;
dest = sconfig->dst_addr;
- endpoints = SUN4I_DMA_CFG_SRC_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM) |
- SUN4I_DMA_CFG_DST_DRQ_TYPE(vchan->endpoint) |
- SUN4I_DMA_CFG_DST_ADDR_MODE(SUN4I_NDMA_ADDR_MODE_IO);
+ endpoints = SUN4I_DMA_CFG_DST_DRQ_TYPE(vchan->endpoint) |
+ SUN4I_DMA_CFG_DST_ADDR_MODE(io_mode) |
+ SUN4I_DMA_CFG_SRC_DRQ_TYPE(ram_type);
} else {
src = sconfig->src_addr;
dest = buf;
- endpoints = SUN4I_DMA_CFG_SRC_DRQ_TYPE(vchan->endpoint) |
- SUN4I_DMA_CFG_SRC_ADDR_MODE(SUN4I_NDMA_ADDR_MODE_IO) |
- SUN4I_DMA_CFG_DST_DRQ_TYPE(SUN4I_NDMA_DRQ_TYPE_SDRAM);
+ endpoints = SUN4I_DMA_CFG_DST_DRQ_TYPE(ram_type) |
+ SUN4I_DMA_CFG_SRC_DRQ_TYPE(vchan->endpoint) |
+ SUN4I_DMA_CFG_SRC_ADDR_MODE(io_mode);
}

/*
@@ -747,8 +745,13 @@ sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t len,
dest = buf + offset;

/* Make the promise */
- promise = generate_ndma_promise(chan, src, dest,
- plength, sconfig, dir);
+ if (vchan->is_dedicated)
+ promise = generate_ddma_promise(chan, src, dest,
+ plength, sconfig);
+ else
+ promise = generate_ndma_promise(chan, src, dest,
+ plength, sconfig, dir);
+
if (!promise) {
/* TODO: should we free everything? */
return NULL;
--
2.17.1

[Stefan Mavrodiev]

Add HDMI audio support for the sun4i-hdmi encoder, used on
the older Allwinner chips - A10, A20, A31.

Most of the code is based on the BSP implementation. In it
dditional formats are supported (S20_3LE and S24_LE), however
there where some problems with them and only S16_LE is left.

Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---

drivers/gpu/drm/sun4i/Kconfig | 1 +
drivers/gpu/drm/sun4i/Makefile | 1 +
drivers/gpu/drm/sun4i/sun4i_hdmi.h | 30 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 375 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 +

5 files changed, 411 insertions(+)
create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 37e90e42943f..192b732b10cd 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -19,6 +19,7 @@ if DRM_SUN4I
config DRM_SUN4I_HDMI
tristate "Allwinner A10 HDMI Controller Support"
default DRM_SUN4I
+ select SND_PCM_ELD
help
Choose this option if you have an Allwinner SoC with an HDMI
controller.
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0d04f2447b01..e2d82b451c36 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -5,6 +5,7 @@ sun4i-frontend-y += sun4i_frontend.o
sun4i-drm-y += sun4i_drv.o
sun4i-drm-y += sun4i_framebuffer.o

+sun4i-drm-hdmi-y += sun4i_hdmi_audio.o
sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o
sun4i-drm-hdmi-y += sun4i_hdmi_enc.o
sun4i-drm-hdmi-y += sun4i_hdmi_i2c.o
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
index 7ad3f06c127e..456964e681b0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
@@ -42,7 +42,32 @@
#define SUN4I_HDMI_VID_TIMING_POL_VSYNC BIT(1)
#define SUN4I_HDMI_VID_TIMING_POL_HSYNC BIT(0)

+#define SUN4I_HDMI_AUDIO_CTRL_REG 0x040
+#define SUN4I_HDMI_AUDIO_CTRL_ENABLE BIT(31)
+#define SUN4I_HDMI_AUDIO_CTRL_RESET BIT(30)
+
+#define SUN4I_HDMI_AUDIO_FMT_REG 0x048
+#define SUN4I_HDMI_AUDIO_FMT_SRC BIT(31)
+#define SUN4I_HDMI_AUDIO_FMT_LAYOUT BIT(3)
+#define SUN4I_HDMI_AUDIO_FMT_CH_CFG(n) (n - 1)
+#define SUN4I_HDMI_AUDIO_FMT_CH_CFG_MASK GENMASK(2, 0)
+
+#define SUN4I_HDMI_AUDIO_PCM_REG 0x4c
+#define SUN4I_HDMI_AUDIO_PCM_CH_MAP(n, m) ((m - 1) << (n * 4))
+#define SUN4I_HDMI_AUDIO_PCM_CH_MAP_MASK(n) (GENMASK(2, 0) << (n * 4))
+
+#define SUN4I_HDMI_AUDIO_CTS_REG 0x050
+#define SUN4I_HDMI_AUDIO_CTS(n) (n & GENMASK(19, 0))
+
+#define SUN4I_HDMI_AUDIO_N_REG 0x054
+#define SUN4I_HDMI_AUDIO_N(n) (n & GENMASK(19, 0))
+
+#define SUN4I_HDMI_AUDIO_STAT0_REG 0x58
+#define SUN4I_HDMI_AUDIO_STAT0_FREQ(n) (n << 24)
+#define SUN4I_HDMI_AUDIO_STAT0_FREQ_MASK GENMASK(27, 24)
+
#define SUN4I_HDMI_AVI_INFOFRAME_REG(n) (0x080 + (n))
+#define SUN4I_HDMI_AUDIO_INFOFRAME_REG(n) (0x0a0 + (n))

#define SUN4I_HDMI_PAD_CTRL0_REG 0x200
#define SUN4I_HDMI_PAD_CTRL0_BIASEN BIT(31)
@@ -283,9 +308,13 @@ struct sun4i_hdmi {
struct regmap_field *field_ddc_sda_en;
struct regmap_field *field_ddc_sck_en;

+ u8 hdmi_audio_channels;
+
struct sun4i_drv *drv;

bool hdmi_monitor;
+ bool hdmi_audio;
+
struct cec_adapter *cec_adap;

const struct sun4i_hdmi_variant *variant;
@@ -294,5 +323,6 @@ struct sun4i_hdmi {
int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk);
int sun4i_tmds_create(struct sun4i_hdmi *hdmi);
int sun4i_hdmi_i2c_create(struct device *dev, struct sun4i_hdmi *hdmi);
+int sun4i_hdmi_audio_create(struct sun4i_hdmi *hdmi);

#endif /* _SUN4I_HDMI_H_ */
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
new file mode 100644
index 000000000000..b6d4199d15ce
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Olimex Ltd.
+ * Author: Stefan Mavrodiev <ste...@olimex.com>
+ */
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_print.h>
+
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_drm_eld.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "sun4i_hdmi.h"
+
+static const struct snd_soc_dapm_widget sun4i_hdmi_audio_widgets[] = {
+ SND_SOC_DAPM_OUTPUT("TX"),
+};
+
+static const struct snd_soc_dapm_route sun4i_hdmi_audio_routes[] = {
+ { "TX", NULL, "Playback" },
+};
+
+static const struct snd_soc_component_driver sun4i_hdmi_audio_component = {
+ .dapm_widgets = sun4i_hdmi_audio_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_hdmi_audio_widgets),
+ .dapm_routes = sun4i_hdmi_audio_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_hdmi_audio_routes),
+};
+
+static int sun4i_hdmi_audio_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+ u32 reg;
+ int ret;
+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_RESET);
+ ret = regmap_read_poll_timeout(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ reg, !reg, 100, 50000);
+ if (ret < 0) {
+ DRM_ERROR("Failed to reset HDMI Audio\n");
+ return ret;
+ }
+
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_ENABLE);
+
+ return snd_pcm_hw_constraint_eld(substream->runtime,
+ hdmi->connector.eld);
+}
+
+static void sun4i_hdmi_audio_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+}
+
+static int sun4i_hdmi_setup_audio_infoframes(struct sun4i_hdmi *hdmi)
+{
+ union hdmi_infoframe frame;
+ u8 buffer[14];
+ int i, ret;
+
+ ret = hdmi_audio_infoframe_init(&frame.audio);
+ if (ret < 0) {
+ DRM_ERROR("Failed to init HDMI audio infoframe\n");
+ return ret;
+ }
+
+ 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 = hdmi->hdmi_audio_channels;
+
+ ret = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
+ if (ret < 0) {
+ DRM_ERROR("Failed to pack HDMI audio infoframe\n");
+ return ret;
+ }
+
+ for (i = 0; i < sizeof(buffer); i++)
+ writeb(buffer[i],
+ hdmi->base + SUN4I_HDMI_AUDIO_INFOFRAME_REG(i));
+
+ return 0;
+}
+
+static void sun4i_hdmi_audio_set_cts_n(struct sun4i_hdmi *hdmi,
+ struct snd_pcm_hw_params *params)
+{
+ struct drm_encoder *encoder = &hdmi->encoder;
+ struct drm_crtc *crtc = encoder->crtc;
+ const struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+ u32 rate = params_rate(params);
+ u32 n, cts;
+ u64 tmp;
+
+ /**
+ * Calculate Cycle Time Stamp (CTS) and Numerator (N):
+ *
+ * N = 128 * Samplerate / 1000
+ * CTS = (Ftdms * N) / (128 * Samplerate)
+ */
+
+ n = 128 * rate / 1000;
+ tmp = (u64)(mode->clock * 1000) * n;
+ do_div(tmp, 128 * rate);
+ cts = tmp;
+
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTS_REG,
+ SUN4I_HDMI_AUDIO_CTS(cts));
+
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_N_REG,
+ SUN4I_HDMI_AUDIO_N(n));
+}
+
+static int sun4i_hdmi_audio_set_hw_rate(struct sun4i_hdmi *hdmi,
+ struct snd_pcm_hw_params *params)
+{
+ u32 rate = params_rate(params);
+ u32 val;
+
+ switch (rate) {
+ case 44100:
+ val = 0x0;
+ break;
+ case 48000:
+ val = 0x2;
+ break;
+ case 32000:
+ val = 0x3;
+ break;
+ case 88200:
+ val = 0x8;
+ break;
+ case 96000:
+ val = 0x9;
+ break;
+ case 176400:
+ val = 0xc;
+ break;
+ case 192000:
+ val = 0xe;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_STAT0_REG,
+ SUN4I_HDMI_AUDIO_STAT0_FREQ_MASK,
+ SUN4I_HDMI_AUDIO_STAT0_FREQ(val));
+
+ return 0;
+}
+
+static int sun4i_hdmi_audio_set_hw_channels(struct sun4i_hdmi *hdmi,
+ struct snd_pcm_hw_params *params)
+{
+ u32 channels = params_channels(params);
+
+ if (channels > 8)
+ return -EINVAL;
+
+ hdmi->hdmi_audio_channels = channels;
+
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_FMT_REG,
+ SUN4I_HDMI_AUDIO_FMT_LAYOUT,
+ (channels > 2) ? SUN4I_HDMI_AUDIO_FMT_LAYOUT : 0);
+
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_FMT_REG,
+ SUN4I_HDMI_AUDIO_FMT_CH_CFG_MASK,
+ SUN4I_HDMI_AUDIO_FMT_CH_CFG(channels));
+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_PCM_REG, 0x76543210);
+
+ /**
+ * If only one channel is required, send the same sample
+ * to the sink device as a left and right channel.
+ */
+ if (channels == 1)
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_PCM_REG,
+ SUN4I_HDMI_AUDIO_PCM_CH_MAP_MASK(1),
+ SUN4I_HDMI_AUDIO_PCM_CH_MAP(1, 1));
+
+ return 0;
+}
+
+static int sun4i_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+ int ret;
+
+ ret = sun4i_hdmi_audio_set_hw_rate(hdmi, params);
+ if (ret)
+ return ret;
+
+ ret = sun4i_hdmi_audio_set_hw_channels(hdmi, params);
+ if (ret)
+ return ret;
+
+ sun4i_hdmi_audio_set_cts_n(hdmi, params);
+
+ return 0;
+}
+
+static int sun4i_hdmi_audio_trigger(struct snd_pcm_substream *substream,
+ int cmd,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+ int ret = 0;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ ret = sun4i_hdmi_setup_audio_infoframes(hdmi);
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static const struct snd_soc_dai_ops sun4i_hdmi_audio_dai_ops = {
+ .startup = sun4i_hdmi_audio_startup,
+ .shutdown = sun4i_hdmi_audio_shutdown,
+ .hw_params = sun4i_hdmi_audio_hw_params,
+ .trigger = sun4i_hdmi_audio_trigger,
+};
+
+static int sun4i_hdmi_audio_dai_probe(struct snd_soc_dai *dai)
+{
+ struct snd_dmaengine_dai_dma_data *dma_data;
+
+ dma_data = devm_kzalloc(dai->dev, sizeof(*dma_data), GFP_KERNEL);
+ if (!dma_data)
+ return -ENOMEM;
+
+ dma_data->addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ dma_data->maxburst = 8;
+
+ snd_soc_dai_init_dma_data(dai, dma_data, NULL);
+
+ return 0;
+}
+
+static struct snd_soc_dai_driver sun4i_hdmi_audio_dai = {
+ .name = "HDMI",
+ .ops = &sun4i_hdmi_audio_dai_ops,
+ .probe = sun4i_hdmi_audio_dai_probe,
+ .playback = {
+ .stream_name = "Playback",
+ .channels_min = 1,
+ .channels_max = 8,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .rates = SNDRV_PCM_RATE_8000_192000,
+ },
+};
+
+static const struct snd_pcm_hardware sun4i_hdmi_audio_pcm_hardware = {
+ .info = SNDRV_PCM_INFO_INTERLEAVED |
+ SNDRV_PCM_INFO_BLOCK_TRANSFER |
+ SNDRV_PCM_INFO_MMAP |
+ SNDRV_PCM_INFO_MMAP_VALID |
+ SNDRV_PCM_INFO_PAUSE |
+ SNDRV_PCM_INFO_RESUME,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .rates = SNDRV_PCM_RATE_8000_192000,
+ .rate_min = 8000,
+ .rate_max = 192000,
+ .channels_min = 1,
+ .channels_max = 8,
+ .buffer_bytes_max = 128 * 1024,
+ .period_bytes_min = 4 * 1024,
+ .period_bytes_max = 32 * 1024,
+ .periods_min = 2,
+ .periods_max = 8,
+ .fifo_size = 128,
+};
+
+static const struct snd_dmaengine_pcm_config sun4i_hdmi_audio_pcm_config = {
+ .chan_names[SNDRV_PCM_STREAM_PLAYBACK] = "audio-tx",
+ .pcm_hardware = &sun4i_hdmi_audio_pcm_hardware,
+ .prealloc_buffer_size = 128 * 1024,
+ .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+};
+
+struct snd_soc_card sun4i_hdmi_audio_card = {
+ .name = "sun4i-hdmi",
+};
+
+int sun4i_hdmi_audio_create(struct sun4i_hdmi *hdmi)
+{
+ struct snd_soc_card *card = &sun4i_hdmi_audio_card;
+ struct snd_soc_dai_link_component *comp;
+ struct snd_soc_dai_link *link;
+ int ret;
+
+ ret = devm_snd_dmaengine_pcm_register(hdmi->dev,
+ &sun4i_hdmi_audio_pcm_config, 0);
+ if (ret) {
+ DRM_ERROR("Could not register PCM\n");
+ return ret;
+ }
+
+ ret = devm_snd_soc_register_component(hdmi->dev,
+ &sun4i_hdmi_audio_component,
+ &sun4i_hdmi_audio_dai, 1);
+ if (ret) {
+ DRM_ERROR("Could not register DAI\n");
+ return ret;
+ }
+
+ link = devm_kzalloc(hdmi->dev, sizeof(*link), GFP_KERNEL);
+ if (!link)
+ return -ENOMEM;
+
+ comp = devm_kzalloc(hdmi->dev, sizeof(*comp) * 3, GFP_KERNEL);
+ if (!comp)
+ return -ENOMEM;
+
+ link->cpus = &comp[0];
+ link->codecs = &comp[1];
+ link->platforms = &comp[2];
+
+ link->num_cpus = 1;
+ link->num_codecs = 1;
+ link->num_platforms = 1;
+
+ link->playback_only = 1;
+
+ link->name = "SUN4I-HDMI";
+ link->stream_name = "SUN4I-HDMI PCM";
+
+ link->codecs->name = dev_name(hdmi->dev);
+ link->codecs->dai_name = sun4i_hdmi_audio_dai.name;
+
+ link->cpus->dai_name = dev_name(hdmi->dev);
+
+ link->platforms->name = dev_name(hdmi->dev);
+
+ link->dai_fmt = SND_SOC_DAIFMT_I2S;
+
+ card->dai_link = link;
+ card->num_links = 1;
+ card->dev = hdmi->dev;
+
+ snd_soc_card_set_drvdata(card, hdmi);
+ return devm_snd_soc_register_card(hdmi->dev, card);
+}
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index a7c4654445c7..79ecd89fb705 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -114,6 +114,9 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder)
val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;

writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
+
+ if (hdmi->hdmi_audio && sun4i_hdmi_audio_create(hdmi))
+ DRM_ERROR("Couldn't create the HDMI audio adapter\n");
}

static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,
@@ -218,6 +221,7 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector)
if (!edid)
return 0;

+ hdmi->hdmi_audio = drm_detect_monitor_audio(edid);
hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
--
2.17.1

[Chen-Yu Tsai]

I would drop the ASoC stuff and just do a standard ALSA driver.
You really don't gain anything from using ASoC, since this is
just a really standard PCM DMA interface plus some controls.
There aren't multiple components that need to be strung together.

ChenYu

[Stefan Mavrodiev]

Hi,

On 1/10/20 6:26 PM, Maxime Ripard wrote:
> Hi,

> There's the issue multiple times in the headers, but you should wrap n
> in parentheses to make sure we have no issue with precedence when
> calling the macro.

> Out of curiosity, did you try to remove the module with that patch
> applied? IIRC, these functions will overwrite the device drvdata, and
> we will try to access them in unbind / remove.
Actually I did not. Just tried that and you're right. The module crashes
at the unbind call.
I use sun4i_hdmi struct only for regmap. Maybe create separate private
structure and copy only
regmap?

>
>> +}
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>> index a7c4654445c7..79ecd89fb705 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>> @@ -114,6 +114,9 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder)
>> val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;
>>
>> writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
>> +
>> + if (hdmi->hdmi_audio && sun4i_hdmi_audio_create(hdmi))
>> + DRM_ERROR("Couldn't create the HDMI audio adapter\n");

> So you create the audio card each time the display is enabled? I guess
> this is to deal with the hotplug?
Yes. See below.
>
> I'm not sure this is the right thing to do. If I remember well, the
> ELD are here precisely to let userspace know that the display is
> plugged (and audio-capable) or not.
>
> Also, you don't remove that card in the disable, which mean that if
> you end up in a situation where you would enable the display, disable
> it and then enable it again, you have two audio cards now.
There is issue with the hotplug. When inserting the cable, the event is
detected
and the hdmi encoder is enabled. Thus the card is created. However
further removal and
insertions are not detected. This is why I don't remove the card.

Also I count on devm_snd_soc_register_card() to release the card.
>
> Thanks!
> Maxime


Best regards,
Stefan

[Stefan Mavrodiev]

Hi,

On 1/15/20 10:32 AM, Maxime Ripard wrote:
> Hi Stefan,

> I think the issue is that:
>
> - In bind, we first call dev_set_drvdata on the bound device, with a
> pointer to struct sun4i_hdmi as the value. The driver_data field
> in struct device is now a pointer to our instance of struct
> sun4i_hdmi.
>
> - In audio create, you then call snd_soc_card_set_drvdata with a
> pointer to struct sun4i_hdmi as the value. The drvdata field in
> the struct snd_soc_card is now a pointer to our instance of struct
> sun4i_hdmi (so far so good).
>
> - Then you call (devm_)snd_soc_register_card. One of the thing that
> it will do is call drv_set_drvdata on the card->dev device,
> setting it to our pointer to the struct snd_soc_card we provided.
> However, since you set card->dev to the same device than the one
> initially bound, this means that you just overwrote the struct
> sun4i_hdmi pointer with a pointer to struct snd_soc_card.
>
> - The driver will operate properly, since we never really use the
> driver_data field, in the HDMI driver, except when...
>
> - At unbind, you retrieve the driver_data field, expecting a struct
> sun4i_hdmi pointer, except you have a pointer to struct
> snd_soc_card, and everything explodes.
>
> I think the way to work around that would be to create a new
> (platform_)device for the HDMI audio component, so that ASoC can work
> on that device instead.
>
> This seems to be what dw-hdmi is doing here:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c#L2812
>
> (Except that they are also using platform_data, since they have
> multiple drivers, we wouldn't, so we can just lookup sun4i_hdmi using
> the parent's device driver_data).

>
>>>> +}
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>>>> index a7c4654445c7..79ecd89fb705 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>>>> @@ -114,6 +114,9 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder)
>>>> val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;
>>>>
>>>> writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
>>>> +
>>>> + if (hdmi->hdmi_audio && sun4i_hdmi_audio_create(hdmi))
>>>> + DRM_ERROR("Couldn't create the HDMI audio adapter\n");
>>> So you create the audio card each time the display is enabled? I guess
>>> this is to deal with the hotplug?
>> Yes. See below.
>>
>>> I'm not sure this is the right thing to do. If I remember well, the
>>> ELD are here precisely to let userspace know that the display is
>>> plugged (and audio-capable) or not.
>>>
>>> Also, you don't remove that card in the disable, which mean that if
>>> you end up in a situation where you would enable the display, disable
>>> it and then enable it again, you have two audio cards now.
>> There is issue with the hotplug. When inserting the cable, the event
>> is detected and the hdmi encoder is enabled. Thus the card is
>> created. However further removal and insertions are not
>> detected.

> I guess we would need to fix that then?

>
>> This is why I don't remove the card.
>>
>> Also I count on devm_snd_soc_register_card() to release the card.

> I think you should really create the card all the time, and just
> update the ELD to let the userspace know when something has been
> created.
>
> And yeah, we should have a working hotplug, but that's a separate
> story :)

Thank you for the review. Soon I'll prepare v2.

Also I'll check the hotplug issue.

>
> Maxime

Best regards,
Stefan

[Stefan Mavrodiev]

This patch series add support for HDMI audio for sun4i HDMI encored.
The code uses some parts from the Allwinners's BSP kernel.

Currently cyclic DMA transfers are disabled. The first patch permits them
as they are required for the audio.

The patch is tested on A20 chip. For the other chips, only the addresses
of the registers are checked.

Changes for v2:
- Create a new platform driver instead of using the HDMI encoder
- Expose a new kcontrol to the userspace holding the ELD data
- Wrap all macro arguments in parentheses

Stefan Mavrodiev (2):
dmaengine: sun4i: Add support for cyclic requests with dedicated DMA
drm: sun4i: hdmi: Add support for sun4i HDMI encoder audio

drivers/dma/sun4i-dma.c | 45 +--

drivers/gpu/drm/sun4i/Kconfig | 1 +
drivers/gpu/drm/sun4i/Makefile | 1 +

drivers/gpu/drm/sun4i/sun4i_hdmi.h | 28 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 452 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 20 +
6 files changed, 526 insertions(+), 21 deletions(-)

[Stefan Mavrodiev]

Currently the cyclic transfers can be used only with normal DMAs. They
can be used by pcm_dmaengine module, which is required for implementing
sound with sun4i-hdmi encoder. This is so because the controller can
accept audio only from a dedicated DMA.

This patch enables them, following the existing style for the
scatter/gather type transfers.

Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---

Changes for v2:
- None

[Stefan Mavrodiev]

Add HDMI audio support for the sun4i-hdmi encoder, used on
the older Allwinner chips - A10, A20, A31.

Most of the code is based on the BSP implementation. In it
dditional formats are supported (S20_3LE and S24_LE), however
there where some problems with them and only S16_LE is left.

Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---

Changes for v2:
- Create a new platform driver instead of using the HDMI encoder
- Expose a new kcontrol to the userspace holding the ELD data
- Wrap all macro arguments in parentheses

drivers/gpu/drm/sun4i/Kconfig | 1 +
drivers/gpu/drm/sun4i/Makefile | 1 +
drivers/gpu/drm/sun4i/sun4i_hdmi.h | 28 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 452 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 20 +

5 files changed, 502 insertions(+)

index 7ad3f06c127e..a965a97b4814 100644

--- a/drivers/gpu/drm/sun4i/sun4i_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
@@ -42,7 +42,32 @@
#define SUN4I_HDMI_VID_TIMING_POL_VSYNC BIT(1)
#define SUN4I_HDMI_VID_TIMING_POL_HSYNC BIT(0)

+#define SUN4I_HDMI_AUDIO_CTRL_REG 0x040
+#define SUN4I_HDMI_AUDIO_CTRL_ENABLE BIT(31)
+#define SUN4I_HDMI_AUDIO_CTRL_RESET BIT(30)
+
+#define SUN4I_HDMI_AUDIO_FMT_REG 0x048
+#define SUN4I_HDMI_AUDIO_FMT_SRC BIT(31)
+#define SUN4I_HDMI_AUDIO_FMT_LAYOUT BIT(3)

+#define SUN4I_HDMI_AUDIO_FMT_CH_CFG(n) ((n) - 1)

+#define SUN4I_HDMI_AUDIO_FMT_CH_CFG_MASK GENMASK(2, 0)
+
+#define SUN4I_HDMI_AUDIO_PCM_REG 0x4c

+#define SUN4I_HDMI_AUDIO_PCM_CH_MAP(n, m) (((m) - 1) << ((n) * 4))
+#define SUN4I_HDMI_AUDIO_PCM_CH_MAP_MASK(n) (GENMASK(2, 0) << ((n) * 4))
+
+#define SUN4I_HDMI_AUDIO_CTS_REG 0x050
+#define SUN4I_HDMI_AUDIO_CTS(n) ((n) & GENMASK(19, 0))
+
+#define SUN4I_HDMI_AUDIO_N_REG 0x054
+#define SUN4I_HDMI_AUDIO_N(n) ((n) & GENMASK(19, 0))
+
+#define SUN4I_HDMI_AUDIO_STAT0_REG 0x58
+#define SUN4I_HDMI_AUDIO_STAT0_FREQ(n) ((n) << 24)

+#define SUN4I_HDMI_AUDIO_STAT0_FREQ_MASK GENMASK(27, 24)
+
#define SUN4I_HDMI_AVI_INFOFRAME_REG(n) (0x080 + (n))
+#define SUN4I_HDMI_AUDIO_INFOFRAME_REG(n) (0x0a0 + (n))

#define SUN4I_HDMI_PAD_CTRL0_REG 0x200
#define SUN4I_HDMI_PAD_CTRL0_BIASEN BIT(31)

@@ -286,9 +311,12 @@ struct sun4i_hdmi {

struct sun4i_drv *drv;

bool hdmi_monitor;
+ bool hdmi_audio;
+
struct cec_adapter *cec_adap;

const struct sun4i_hdmi_variant *variant;

+ struct platform_device *audio;
};

int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk);

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
new file mode 100644

index 000000000000..fbfe59459633
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
@@ -0,0 +1,452 @@

+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Olimex Ltd.
+ * Author: Stefan Mavrodiev <ste...@olimex.com>
+ */
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_print.h>
+
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_drm_eld.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>

+
+#include "sun4i_hdmi.h"
+
+#define DRIVER_NAME "sun4i-hdmi-audio"
+
+struct sun4i_hdmi_audio {
+ struct device *hdmi;
+ u8 audio_channels;
+};
+
+static int sun4i_hdmi_audio_ctl_eld_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+ uinfo->count = MAX_ELD_BYTES;

+ return 0;
+}
+

+static int sun4i_hdmi_audio_ctl_eld_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+ struct snd_soc_card *card = snd_soc_component_get_drvdata(component);
+ struct sun4i_hdmi_audio *priv = snd_soc_card_get_drvdata(card);
+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);
+
+ memcpy(ucontrol->value.bytes.data,
+ hdmi->connector.eld,
+ MAX_ELD_BYTES);

+
+ return 0;
+}
+

+static const struct snd_kcontrol_new sun4i_hdmi_audio_controls[] = {
+ {
+ .access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = "ELD",
+ .info = sun4i_hdmi_audio_ctl_eld_info,
+ .get = sun4i_hdmi_audio_ctl_eld_get,
+ },
+};
+

+static const struct snd_soc_dapm_widget sun4i_hdmi_audio_widgets[] = {
+ SND_SOC_DAPM_OUTPUT("TX"),
+};
+
+static const struct snd_soc_dapm_route sun4i_hdmi_audio_routes[] = {
+ { "TX", NULL, "Playback" },
+};
+
+static const struct snd_soc_component_driver sun4i_hdmi_audio_component = {

+ .controls = sun4i_hdmi_audio_controls,
+ .num_controls = ARRAY_SIZE(sun4i_hdmi_audio_controls),

+ .dapm_widgets = sun4i_hdmi_audio_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_hdmi_audio_widgets),
+ .dapm_routes = sun4i_hdmi_audio_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_hdmi_audio_routes),
+};
+
+static int sun4i_hdmi_audio_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi_audio *priv = snd_soc_card_get_drvdata(card);
+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);
+ u32 reg;
+ int ret;
+

+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_RESET);
+ ret = regmap_read_poll_timeout(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ reg, !reg, 100, 50000);
+ if (ret < 0) {

+ DRM_ERROR("Failed to reset HDMI Audio\n");

+ return ret;
+ }
+

+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_ENABLE);
+
+ return snd_pcm_hw_constraint_eld(substream->runtime,
+ hdmi->connector.eld);
+}
+
+static void sun4i_hdmi_audio_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi_audio *priv = snd_soc_card_get_drvdata(card);
+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);

+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+}
+

+static int sun4i_hdmi_setup_audio_infoframes(struct sun4i_hdmi_audio *priv)
+{
+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);

+ union hdmi_infoframe frame;
+ u8 buffer[14];

+ int i, ret;
+

+ ret = hdmi_audio_infoframe_init(&frame.audio);
+ if (ret < 0) {

+ DRM_ERROR("Failed to init HDMI audio infoframe\n");

+ return ret;
+ }
+

+ 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 = priv->audio_channels;

+
+ ret = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
+ if (ret < 0) {

+ DRM_ERROR("Failed to pack HDMI audio infoframe\n");

+ return ret;
+ }
+

+ for (i = 0; i < sizeof(buffer); i++)
+ writeb(buffer[i],
+ hdmi->base + SUN4I_HDMI_AUDIO_INFOFRAME_REG(i));
+
+ return 0;
+}
+

+static void sun4i_hdmi_audio_set_cts_n(struct sun4i_hdmi_audio *priv,

+ struct snd_pcm_hw_params *params)
+{

+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);

+static int sun4i_hdmi_audio_set_hw_rate(struct sun4i_hdmi_audio *priv,

+ struct snd_pcm_hw_params *params)
+{

+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);

+static int sun4i_hdmi_audio_set_hw_channels(struct sun4i_hdmi_audio *priv,

+ struct snd_pcm_hw_params *params)
+{

+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);

+ u32 channels = params_channels(params);
+
+ if (channels > 8)
+ return -EINVAL;
+

+ priv->audio_channels = channels;

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi_audio *priv = snd_soc_card_get_drvdata(card);
+ int ret;
+
+ ret = sun4i_hdmi_audio_set_hw_rate(priv, params);
+ if (ret)
+ return ret;
+
+ ret = sun4i_hdmi_audio_set_hw_channels(priv, params);
+ if (ret)
+ return ret;
+
+ sun4i_hdmi_audio_set_cts_n(priv, params);

+
+ return 0;
+}
+
+static int sun4i_hdmi_audio_trigger(struct snd_pcm_substream *substream,
+ int cmd,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi_audio *priv = snd_soc_card_get_drvdata(card);

+ int ret = 0;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:

+ ret = sun4i_hdmi_setup_audio_infoframes(priv);

+ break;
+ default:
+ break;
+ }
+

+ return ret;
+}
+

+static const struct snd_soc_dai_ops sun4i_hdmi_audio_dai_ops = {
+ .startup = sun4i_hdmi_audio_startup,
+ .shutdown = sun4i_hdmi_audio_shutdown,
+ .hw_params = sun4i_hdmi_audio_hw_params,
+ .trigger = sun4i_hdmi_audio_trigger,
+};
+
+static int sun4i_hdmi_audio_dai_probe(struct snd_soc_dai *dai)
+{
+ struct snd_dmaengine_dai_dma_data *dma_data;
+
+ dma_data = devm_kzalloc(dai->dev, sizeof(*dma_data), GFP_KERNEL);
+ if (!dma_data)

+ return -ENOMEM;
+

+static int sun4i_hdmi_audio_probe(struct platform_device *pdev)

+{
+ struct snd_soc_card *card = &sun4i_hdmi_audio_card;
+ struct snd_soc_dai_link_component *comp;
+ struct snd_soc_dai_link *link;

+ struct device *dev = &pdev->dev;
+ struct sun4i_hdmi_audio *priv;
+ int ret;
+
+ ret = devm_snd_dmaengine_pcm_register(dev,

+ &sun4i_hdmi_audio_pcm_config, 0);
+ if (ret) {

+ dev_err(dev, "Failed registering PCM DMA component\n");

+ return ret;
+ }
+

+ ret = devm_snd_soc_register_component(dev,

+ &sun4i_hdmi_audio_component,
+ &sun4i_hdmi_audio_dai, 1);
+ if (ret) {

+ dev_err(dev, "Failed registering DAI component\n");

+ return ret;
+ }
+

+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->hdmi = dev->parent;
+ dev->of_node = dev->parent->of_node;
+
+ link = devm_kzalloc(dev, sizeof(*link), GFP_KERNEL);

+ if (!link)
+ return -ENOMEM;
+

+ comp = devm_kzalloc(dev, sizeof(*comp) * 3, GFP_KERNEL);

+ if (!comp)
+ return -ENOMEM;
+
+ link->cpus = &comp[0];
+ link->codecs = &comp[1];
+ link->platforms = &comp[2];
+
+ link->num_cpus = 1;
+ link->num_codecs = 1;
+ link->num_platforms = 1;
+
+ link->playback_only = 1;
+
+ link->name = "SUN4I-HDMI";
+ link->stream_name = "SUN4I-HDMI PCM";
+

+ link->codecs->name = dev_name(dev);

+ link->codecs->dai_name = sun4i_hdmi_audio_dai.name;
+

+ link->cpus->dai_name = dev_name(dev);
+
+ link->platforms->name = dev_name(dev);

+
+ link->dai_fmt = SND_SOC_DAIFMT_I2S;
+
+ card->dai_link = link;
+ card->num_links = 1;

+ card->dev = dev;
+
+ snd_soc_card_set_drvdata(card, priv);
+ return devm_snd_soc_register_card(dev, card);
+}
+
+static int sun4i_hdmi_audio_remove(struct platform_device *pdev)

+{
+ return 0;
+}
+

+static struct platform_driver sun4i_hdmi_audio_driver = {
+ .probe = sun4i_hdmi_audio_probe,
+ .remove = sun4i_hdmi_audio_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ },
+};
+module_platform_driver(sun4i_hdmi_audio_driver);
+
+MODULE_AUTHOR("Stefan Mavrodiev <ste...@olimex.com");
+MODULE_DESCRIPTION("Allwinner A10 HDMI Audio driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 68d4644ac2dc..ec598b5a1d3a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -92,6 +92,11 @@ static void sun4i_hdmi_disable(struct drm_encoder *encoder)

writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);

clk_disable_unprepare(hdmi->tmds_clk);
+
+ if (hdmi->audio) {
+ platform_device_unregister(hdmi->audio);
+ hdmi->audio = NULL;
+ }

}

static void sun4i_hdmi_enable(struct drm_encoder *encoder)

@@ -114,6 +119,20 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder)

val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;

writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
+

+ if (hdmi->hdmi_audio) {
+ struct platform_device_info pdevinfo;
+
+ memset(&pdevinfo, 0, sizeof(pdevinfo));
+ pdevinfo.name = "sun4i-hdmi-audio";
+ pdevinfo.parent = hdmi->dev;
+ pdevinfo.id = PLATFORM_DEVID_AUTO;
+ hdmi->audio = platform_device_register_full(&pdevinfo);
+ if (IS_ERR(hdmi->audio)) {

+ DRM_ERROR("Couldn't create the HDMI audio adapter\n");

+ hdmi->audio = NULL;
+ }
+ }

}

static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,

@@ -218,6 +237,7 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector)

[Stefan Mavrodiev]

On 1/21/20 10:35 AM, Vinod Koul wrote:
> On 15-01-20, 18:07, Maxime Ripard wrote:

>> On Wed, Jan 15, 2020 at 06:01:37PM +0530, Vinod Koul wrote:
>>> On 10-01-20, 16:11, Stefan Mavrodiev wrote:
>>>> Currently the cyclic transfers can be used only with normal DMAs. They
>>>> can be used by pcm_dmaengine module, which is required for implementing
>>>> sound with sun4i-hdmi encoder. This is so because the controller can
>>>> accept audio only from a dedicated DMA.
>>>>
>>>> This patch enables them, following the existing style for the
>>>> scatter/gather type transfers.

>>> I presume you want this to go with drm tree (if not let me know) so:
>>>
>>> Acked-by: Vinod Koul <vk...@kernel.org>
>> There's no need for it to go through DRM, it can go through your tree :)
> okay in that case I have applied now :), thanks
>
Hi,

Should I keep this patch in the future series or drop it?

Best regards,
Stefan Mavrodiev

[Mark Brown]

On Tue, Jan 21, 2020 at 07:29:37PM +0100, Maxime Ripard wrote:

> > Mark, our issue here is that we have a driver tied to a device that is
> > an HDMI encoder. Obviously, we'll want to register into DRM, which is
> > what we were doing so far, with the usual case where at remove /
> > unbind time, in order to free the resources, we just retrieve our
> > pointer to our private structure using the device's drvdata.

> > Now, snd_soc_register_card also sets that pointer to the card we try
> > to register, which is problematic. It seems that it's used to handle
> > suspend / resume automatically, which in this case would be also not
> > really fit for us (or rather, we would need to do more that just
> > suspend the audio part).

There's a drvdata field in the snd_soc_card for cases like this - would
that work for you?

[Stefan Mavrodiev]

On 1/21/20 8:29 PM, Maxime Ripard wrote:
> +Mark

>
> On Mon, Jan 20, 2020 at 02:33:26PM +0200, Stefan Mavrodiev wrote:
>> Add HDMI audio support for the sun4i-hdmi encoder, used on
>> the older Allwinner chips - A10, A20, A31.
>>
>> Most of the code is based on the BSP implementation. In it
>> dditional formats are supported (S20_3LE and S24_LE), however
>> there where some problems with them and only S16_LE is left.

> What are those problems?
It was all noise. Guess it's some byte alignment issue, but
I didn't manage to solve it.

>
>> Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
>> ---

> Sorry if I wasn't clear enough in the previous mail, I didn't suggest
> to do a driver, this will open another can of worm (as kbuild already
> pointed out), but to create a new device, and pass that new device to
> ASoC's functions.

The problem here is that I used sunxi_defconfig instead of the arch
defconfig during testing. The difference is that in "our" config the
modules are built-in and thus there is no module_init.

However I've managed to get it working in v3 (it's not submitted
yet).

I've added separate entry in the Kconfig of type bool. This is to
manage the SND_SOC dependency. The current patch will fail if
SND_SOC=m and SUN4I_HDMI=y for example.

Then I've dropped this as it's useless:

> +module_platform_driver(sun4i_hdmi_audio_driver);
> +
> +MODULE_AUTHOR("Stefan Mavrodiev <ste...@olimex.com");
> +MODULE_DESCRIPTION("Allwinner A10 HDMI Audio driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);

In sun4i_hdmi_enc.c I've added __init and __exit
functions where I register/unregister the additional platform_driver.
Thus we get 2 platform drivers from one module, which is not something
uncommon.

If you like this approach I can submit v3.

>
> I tried that, and failed, so I guess it's not an option either.

>
> Mark, our issue here is that we have a driver tied to a device that is
> an HDMI encoder. Obviously, we'll want to register into DRM, which is
> what we were doing so far, with the usual case where at remove /
> unbind time, in order to free the resources, we just retrieve our
> pointer to our private structure using the device's drvdata.
>
> Now, snd_soc_register_card also sets that pointer to the card we try
> to register, which is problematic. It seems that it's used to handle
> suspend / resume automatically, which in this case would be also not
> really fit for us (or rather, we would need to do more that just
> suspend the audio part).
>

> Is there anyway we can have that kind of setup? I believe vc4 is in a
> similar situation, but they worked around it by storing the data they
> want to access in a global pointer, but that only works for one device
> which doesn't really suit us either.
>
> Any suggestions?
> Thanks!
> Maxime

[Stefan Mavrodiev]

Add HDMI audio support for the sun4i-hdmi encoder, used on
the older Allwinner chips - A10, A20, A31.

Most of the code is based on the BSP implementation. In it
dditional formats are supported (S20_3LE and S24_LE), however
there where some problems with them and only S16_LE is left.

Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
---

Changes for v3:
- Instead of platfrom_driver dynammicly register/unregister card
- Add Kconfig dependencies
- Restrore drvdata after card unregistering

Changes for v2:
- Create a new platform driver instead of using the HDMI encoder
- Expose a new kcontrol to the userspace holding the ELD data
- Wrap all macro arguments in parentheses

drivers/gpu/drm/sun4i/Kconfig | 11 +
drivers/gpu/drm/sun4i/Makefile | 3 +
drivers/gpu/drm/sun4i/sun4i_hdmi.h | 37 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 450 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 14 +
5 files changed, 515 insertions(+)

create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig

index 37e90e42943f..ca2ab5d53dd4 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -23,6 +23,17 @@ config DRM_SUN4I_HDMI

Choose this option if you have an Allwinner SoC with an HDMI
controller.

+config DRM_SUN4I_HDMI_AUDIO
+ bool "Allwinner A10 HDMI Audio Support"
+ default y
+ depends on DRM_SUN4I_HDMI
+ depends on SND_SOC=y || SND_SOC=DRM_SUN4I_HDMI
+ select SND_PCM_ELD
+ select SND_SOC_GENERIC_DMAENGINE_PCM
+ help
+ Choose this option if you have an Allwinner SoC with an HDMI
+ controller and want to use audio.
+
config DRM_SUN4I_HDMI_CEC
bool "Allwinner A10 HDMI CEC Support"
depends on DRM_SUN4I_HDMI
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 0d04f2447b01..492bfd28ad2e 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -5,6 +5,9 @@ sun4i-frontend-y += sun4i_frontend.o

sun4i-drm-y += sun4i_drv.o
sun4i-drm-y += sun4i_framebuffer.o

+ifdef CONFIG_DRM_SUN4I_HDMI_AUDIO
+sun4i-drm-hdmi-y += sun4i_hdmi_audio.o
+endif

sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o
sun4i-drm-hdmi-y += sun4i_hdmi_enc.o
sun4i-drm-hdmi-y += sun4i_hdmi_i2c.o
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h

index 7ad3f06c127e..28621d289655 100644

@@ -242,6 +267,11 @@ struct sun4i_hdmi_variant {
bool ddc_fifo_has_dir;
};

+struct sun4i_hdmi_audio {
+ struct snd_soc_card *card;
+ u8 channels;
+};
+
struct sun4i_hdmi {
struct drm_connector connector;
struct drm_encoder encoder;
@@ -283,9 +313,14 @@ struct sun4i_hdmi {

struct regmap_field *field_ddc_sda_en;
struct regmap_field *field_ddc_sck_en;

+

struct sun4i_drv *drv;

bool hdmi_monitor;
+ bool hdmi_audio;
+

+ struct sun4i_hdmi_audio audio;

+
struct cec_adapter *cec_adap;

const struct sun4i_hdmi_variant *variant;

@@ -294,5 +329,7 @@ struct sun4i_hdmi {

int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk);

int sun4i_tmds_create(struct sun4i_hdmi *hdmi);
int sun4i_hdmi_i2c_create(struct device *dev, struct sun4i_hdmi *hdmi);
+int sun4i_hdmi_audio_create(struct sun4i_hdmi *hdmi);

+void sun4i_hdmi_audio_destroy(struct sun4i_hdmi *hdmi);

#endif /* _SUN4I_HDMI_H_ */

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
new file mode 100644

index 000000000000..f42f2cea4e9e
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
@@ -0,0 +1,450 @@

+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Olimex Ltd.
+ * Author: Stefan Mavrodiev <ste...@olimex.com>
+ */
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/of_dma.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_print.h>
+
+#include <sound/dmaengine_pcm.h>
+#include <sound/pcm_drm_eld.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "sun4i_hdmi.h"
+

+static int sun4i_hdmi_audio_ctl_eld_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+ uinfo->count = MAX_ELD_BYTES;

+ return 0;
+}
+

+static int sun4i_hdmi_audio_ctl_eld_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);

+ struct snd_soc_card *card = snd_soc_component_get_drvdata(component);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);

+
+ memcpy(ucontrol->value.bytes.data,
+ hdmi->connector.eld,
+ MAX_ELD_BYTES);

+
+ return 0;
+}
+

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+ u32 reg;
+ int ret;
+

+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_RESET);
+ ret = regmap_read_poll_timeout(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ reg, !reg, 100, 50000);
+ if (ret < 0) {

+ DRM_ERROR("Failed to reset HDMI Audio\n");

+ return ret;
+ }
+

+ regmap_write(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_CTRL_REG,
+ SUN4I_HDMI_AUDIO_CTRL_ENABLE);
+
+ return snd_pcm_hw_constraint_eld(substream->runtime,
+ hdmi->connector.eld);
+}
+
+static void sun4i_hdmi_audio_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);

+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_CTRL_REG, 0);
+}
+

+static int sun4i_hdmi_setup_audio_infoframes(struct sun4i_hdmi *hdmi)
+{

+ union hdmi_infoframe frame;
+ u8 buffer[14];

+ int i, ret;
+

+ ret = hdmi_audio_infoframe_init(&frame.audio);
+ if (ret < 0) {

+ DRM_ERROR("Failed to init HDMI audio infoframe\n");

+ return ret;
+ }
+

+ 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 = hdmi->audio.channels;

+
+ ret = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
+ if (ret < 0) {

+ DRM_ERROR("Failed to pack HDMI audio infoframe\n");

+ return ret;
+ }
+

+ for (i = 0; i < sizeof(buffer); i++)
+ writeb(buffer[i],
+ hdmi->base + SUN4I_HDMI_AUDIO_INFOFRAME_REG(i));

+
+ return 0;
+}
+

+static void sun4i_hdmi_audio_set_cts_n(struct sun4i_hdmi *hdmi,

+ struct snd_pcm_hw_params *params)
+{

+static int sun4i_hdmi_audio_set_hw_rate(struct sun4i_hdmi *hdmi,

+ struct snd_pcm_hw_params *params)
+{

+
+ return 0;
+}
+

+static int sun4i_hdmi_audio_set_hw_channels(struct sun4i_hdmi *hdmi,

+ struct snd_pcm_hw_params *params)
+{

+ u32 channels = params_channels(params);
+
+ if (channels > 8)
+ return -EINVAL;
+

+ hdmi->audio.channels = channels;

+
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_FMT_REG,
+ SUN4I_HDMI_AUDIO_FMT_LAYOUT,
+ (channels > 2) ? SUN4I_HDMI_AUDIO_FMT_LAYOUT : 0);
+
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_FMT_REG,
+ SUN4I_HDMI_AUDIO_FMT_CH_CFG_MASK,
+ SUN4I_HDMI_AUDIO_FMT_CH_CFG(channels));
+
+ regmap_write(hdmi->regmap, SUN4I_HDMI_AUDIO_PCM_REG, 0x76543210);
+
+ /**
+ * If only one channel is required, send the same sample
+ * to the sink device as a left and right channel.
+ */
+ if (channels == 1)
+ regmap_update_bits(hdmi->regmap,
+ SUN4I_HDMI_AUDIO_PCM_REG,
+ SUN4I_HDMI_AUDIO_PCM_CH_MAP_MASK(1),
+ SUN4I_HDMI_AUDIO_PCM_CH_MAP(1, 1));

+
+ return 0;
+}
+

+static int sun4i_hdmi_audio_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);
+ int ret;
+
+ ret = sun4i_hdmi_audio_set_hw_rate(hdmi, params);
+ if (ret < 0)
+ return ret;
+

+ ret = sun4i_hdmi_audio_set_hw_channels(hdmi, params);

+ if (ret < 0)
+ return ret;
+
+ sun4i_hdmi_audio_set_cts_n(hdmi, params);

+
+ return 0;
+}
+

+static int sun4i_hdmi_audio_trigger(struct snd_pcm_substream *substream,
+ int cmd,
+ struct snd_soc_dai *dai)

+{
+ struct snd_soc_card *card = snd_soc_dai_get_drvdata(dai);
+ struct sun4i_hdmi *hdmi = snd_soc_card_get_drvdata(card);

+ int ret = 0;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:

+ ret = sun4i_hdmi_setup_audio_infoframes(hdmi);

+ break;
+ default:
+ break;
+ }
+

+ return ret;
+}
+

+static const struct snd_soc_dai_ops sun4i_hdmi_audio_dai_ops = {
+ .startup = sun4i_hdmi_audio_startup,
+ .shutdown = sun4i_hdmi_audio_shutdown,
+ .hw_params = sun4i_hdmi_audio_hw_params,
+ .trigger = sun4i_hdmi_audio_trigger,
+};
+
+static int sun4i_hdmi_audio_dai_probe(struct snd_soc_dai *dai)
+{
+ struct snd_dmaengine_dai_dma_data *dma_data;
+
+ dma_data = devm_kzalloc(dai->dev, sizeof(*dma_data), GFP_KERNEL);
+ if (!dma_data)

+ return -ENOMEM;
+

+ dma_data->addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ dma_data->maxburst = 8;
+
+ snd_soc_dai_init_dma_data(dai, dma_data, NULL);

+
+ return 0;
+}
+

+int sun4i_hdmi_audio_create(struct sun4i_hdmi *hdmi)

+{
+ struct snd_soc_card *card = &sun4i_hdmi_audio_card;
+ struct snd_soc_dai_link_component *comp;
+ struct snd_soc_dai_link *link;

+ int ret;
+
+ ret = snd_dmaengine_pcm_register(hdmi->dev,
+ &sun4i_hdmi_audio_pcm_config, 0);
+ if (ret < 0) {
+ DRM_ERROR("Could not register PCM\n");

+ return ret;
+ }
+

+ ret = snd_soc_register_component(hdmi->dev,

+ &sun4i_hdmi_audio_component,
+ &sun4i_hdmi_audio_dai, 1);

+ if (ret < 0) {

+ DRM_ERROR("Could not register DAI\n");

+ goto unregister_pcm;
+ }
+
+ link = devm_kzalloc(hdmi->dev, sizeof(*link), GFP_KERNEL);
+ if (!link) {
+ ret = -ENOMEM;
+ goto unregister_component;
+ }
+
+ comp = devm_kzalloc(hdmi->dev, sizeof(*comp) * 3, GFP_KERNEL);
+ if (!comp) {
+ ret = -ENOMEM;
+ goto unregister_component;
+ }

+
+ link->cpus = &comp[0];
+ link->codecs = &comp[1];
+ link->platforms = &comp[2];
+
+ link->num_cpus = 1;
+ link->num_codecs = 1;
+ link->num_platforms = 1;
+
+ link->playback_only = 1;
+
+ link->name = "SUN4I-HDMI";
+ link->stream_name = "SUN4I-HDMI PCM";
+

+ link->codecs->name = dev_name(hdmi->dev);

+ link->codecs->dai_name = sun4i_hdmi_audio_dai.name;
+

+ link->cpus->dai_name = dev_name(hdmi->dev);
+
+ link->platforms->name = dev_name(hdmi->dev);

+
+ link->dai_fmt = SND_SOC_DAIFMT_I2S;
+
+ card->dai_link = link;
+ card->num_links = 1;

+ card->dev = hdmi->dev;
+
+ hdmi->audio.card = card;
+
+ /**
+ * snd_soc_register_card() will overwrite the driver_data pointer.
+ * So before registering the card, store the original pointer in
+ * card->drvdata.
+ */
+ snd_soc_card_set_drvdata(card, hdmi);
+ ret = snd_soc_register_card(card);
+ if (ret)
+ goto unregister_component;

+
+ return 0;
+

+unregister_component:
+ snd_soc_unregister_component(hdmi->dev);
+unregister_pcm:
+ snd_dmaengine_pcm_unregister(hdmi->dev);

+ return ret;
+}
+

+void sun4i_hdmi_audio_destroy(struct sun4i_hdmi *hdmi)
+{
+ struct snd_soc_card *card = hdmi->audio.card;
+ void *data;
+
+ /**
+ * Before removing the card, restore the previously stored driver_data.
+ * This will ensure proper removal of the sun4i-hdmi module, since it
+ * uses dev_get_drvdata() in the unbind function.
+ */
+ data = snd_soc_card_get_drvdata(card);
+
+ snd_soc_unregister_card(card);
+ snd_soc_unregister_component(hdmi->dev);
+ snd_dmaengine_pcm_unregister(hdmi->dev);
+
+ dev_set_drvdata(hdmi->dev, data);
+}
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 68d4644ac2dc..4cd35c97c503 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -23,6 +23,8 @@
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>

+#include <sound/soc.h>
+
#include "sun4i_backend.h"
#include "sun4i_crtc.h"
#include "sun4i_drv.h"
@@ -87,6 +89,10 @@ static void sun4i_hdmi_disable(struct drm_encoder *encoder)

DRM_DEBUG_DRIVER("Disabling the HDMI Output\n");

+#ifdef CONFIG_DRM_SUN4I_HDMI_AUDIO
+ sun4i_hdmi_audio_destroy(hdmi);
+#endif
+
val = readl(hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
val &= ~SUN4I_HDMI_VID_CTRL_ENABLE;

writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);

@@ -114,6 +120,11 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder)

val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;

writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);
+

+#ifdef CONFIG_DRM_SUN4I_HDMI_AUDIO

+ if (hdmi->hdmi_audio && sun4i_hdmi_audio_create(hdmi))

+ DRM_ERROR("Couldn't create the HDMI audio adapter\n");

+#endif

}

static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,

@@ -218,6 +229,9 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector)
if (!edid)
return 0;

+#ifdef CONFIG_DRM_SUN4I_HDMI_AUDIO

+ hdmi->hdmi_audio = drm_detect_monitor_audio(edid);

+#endif

[Stefan Mavrodiev]

This patch series add support for HDMI audio for sun4i HDMI encored.
The code uses some parts from the Allwinners's BSP kernel.

In the previous patch series there was additional patch enabling the
cyclic DMA transfers. It was accepted, so it's omitted here.

The patch is tested on A20 chip. For the other chips, only the addresses
of the registers are checked.

Changes for v3:
- Dropped the patch which enable the cyclic DMA transfers
- In v2, a separate platform_driver was created. However this is not
practical and the idea is dropped.
- Since this module depends on SND_SOC, a new Kconfig entry is added. When
SND_SOC is not enabled, the audio part is not initialized.
- The "big" issue with v1 was that snd_soc_register_card() overwrites
the driver_data pointer. To resolve this issue, the original pointer is
stored as drvdata of the card. Then before card unregistering, the
pointer is restored. This is done by calling additional destroy function.

Changes for v2:
- Create a new platform driver instead of using the HDMI encoder
- Expose a new kcontrol to the userspace holding the ELD data
- Wrap all macro arguments in parentheses

Stefan Mavrodiev (1):

drm: sun4i: hdmi: Add support for sun4i HDMI encoder audio

drivers/gpu/drm/sun4i/Kconfig | 11 +
drivers/gpu/drm/sun4i/Makefile | 3 +
drivers/gpu/drm/sun4i/sun4i_hdmi.h | 37 ++
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 450 +++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 14 +
5 files changed, 515 insertions(+)
create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c

--
2.17.1

[Chen-Yu Tsai]

On Tue, Jan 28, 2020 at 10:07 PM Stefan Mavrodiev <ste...@olimex.com> wrote:
>
> Add HDMI audio support for the sun4i-hdmi encoder, used on
> the older Allwinner chips - A10, A20, A31.
>
> Most of the code is based on the BSP implementation. In it
> dditional formats are supported (S20_3LE and S24_LE), however
> there where some problems with them and only S16_LE is left.
>
> Signed-off-by: Stefan Mavrodiev <ste...@olimex.com>
> ---
> Changes for v3:
> - Instead of platfrom_driver dynammicly register/unregister card
> - Add Kconfig dependencies
> - Restrore drvdata after card unregistering
>
> Changes for v2:
> - Create a new platform driver instead of using the HDMI encoder
> - Expose a new kcontrol to the userspace holding the ELD data
> - Wrap all macro arguments in parentheses
>
> drivers/gpu/drm/sun4i/Kconfig | 11 +
> drivers/gpu/drm/sun4i/Makefile | 3 +
> drivers/gpu/drm/sun4i/sun4i_hdmi.h | 37 ++
> drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c | 450 +++++++++++++++++++++++
> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 14 +
> 5 files changed, 515 insertions(+)
> create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c

Since this is actually an audio driver, please include all the ALSA / ASoC
maintainers and mailing lists (CC-ed).

So using ASoC with all the components IMHO is just adding dead weight. The
audio interface for this particular hardware is just a FIFO that needs to
be written to by an external DMA engine, and a bunch of controls to setup
the parameters of the HDMI audio stream. There's no power sequencing to do,
and no actual individual components to control. There's no reason you couldn't
use just the ALSA DMAENGINE helpers to create a simple ALSA sound card.

(Maybe we could clean it up after it's merged? Would there be any issues
with backward compatibility?)

I think the only example of this besides ASoC is the PXA2xx sound library
and AC97 driver:

- sound/arm/pxa2xx-pcm-lib.c
- sound/arm/pxa2xx-ac97.c

Regards
ChenYu

> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi...@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20200128140642.8404-2-stefan%40olimex.com.

[Stefan Mavrodiev]

Hi,

On 1/29/20 6:43 PM, Maxime Ripard wrote:
> Hi,
>

> I really don't think we should be creating / removing the audio card
> at enable / disable time.

For me it's unnatural to have sound card all the time, even when the HDMI
is not plugged-in.

I'll follow your suggestion. Besides it's easier for me just to drop this
register/unregister mechanism.

Best regards,
Stefan Mavrodiev

>
> To fix the drvdata pointer, you just need to use the card pointer in
> the unbind, and that's it.
>
> Maxime

[Stefan Mavrodiev]

Hi,

I'd like to hear Maxime opinion on this. I get it why it's overkill to
use ASoC.
However it will take only couple of lines to make the changes for v4. On
the other
side with dropping the ASoC, the driver must be almost rewritten. It's
not such a big deal,
but I want to know that this is the right thing to do.

Best regards,
Stefan Mavrodiev


P.S. Sorry for the long break.

[Stefan Mavrodiev]

On 2/25/20 11:58 AM, Maxime Ripard wrote:

>> I'd like to hear Maxime opinion on this. I get it why it's overkill to use
>> ASoC.
>> However it will take only couple of lines to make the changes for v4. On the
>> other
>> side with dropping the ASoC, the driver must be almost rewritten. It's not
>> such a big deal,
>> but I want to know that this is the right thing to do.

> I don't really have an opinion on this, either way is fine for me, so
> if it's simpler to go for ALSA, then let's do it :)

Then ALSA will be. Thanks :)

Stefan

>
> Maxime