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(-)
create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_audio.c
--
2.17.1
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 <[email protected]>
---
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(+)
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..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 <[email protected]>
+ */
+#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);
+ 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_audio *priv,
+ struct snd_pcm_hw_params *params)
+{
+ struct sun4i_hdmi *hdmi = dev_get_drvdata(priv->hdmi);
+ 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_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;
+
+ 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_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;
+
+ 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",
+};
+
+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 <[email protected]");
+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)
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
Hi Stefan,
I love your patch! Yet something to improve:
[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on slave-dma/next v5.5-rc7]
[cannot apply to mripard/sunxi/for-next next-20200120]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Stefan-Mavrodiev/Add-support-for-sun4i-HDMI-audio/20200120-211721
base: https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.o: In function `init_module':
>> sun4i_hdmi_enc.c:(.init.text+0x0): multiple definition of `init_module'
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.o:sun4i_hdmi_audio.c:(.init.text+0x0): first defined here
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.o: In function `cleanup_module':
>> sun4i_hdmi_enc.c:(.exit.text+0x0): multiple definition of `cleanup_module'
drivers/gpu/drm/sun4i/sun4i_hdmi_audio.o:sun4i_hdmi_audio.c:(.exit.text+0x0): first defined here
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
+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?
> Signed-off-by: Stefan Mavrodiev <[email protected]>
> ---
> +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 <[email protected]");
> +MODULE_DESCRIPTION("Allwinner A10 HDMI Audio driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
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.
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
Actually Cc'ing this time..
On Tue, Jan 21, 2020 at 07:29:05PM +0100, 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?
>
> > Signed-off-by: Stefan Mavrodiev <[email protected]>
> > ---
>
> > +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 <[email protected]");
> > +MODULE_DESCRIPTION("Allwinner A10 HDMI Audio driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:" DRIVER_NAME);
>
> 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.
>
> 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
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?
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 <[email protected]>
>> ---
>> +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 <[email protected]");
>> +MODULE_DESCRIPTION("Allwinner A10 HDMI Audio driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
> 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 <[email protected]");
> +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
On Tue, Jan 21, 2020 at 06:32:47PM +0000, Mark Brown wrote:
> 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?
Ah, right, we could just retrieve the snd_soc_card in the unbind, and
the retrieve our structure that way. That's pretty simple :)
Stefan, I guess this is the easiest solution, we should just make sure
that there's a comment to explain why we retrieve snd_soc_card in the
unbind, since it's somewhat unusual.
Thanks!
Maxime