[PATCH v2 2/2] drm: sun4i: hdmi: Add support for sun4i HDMI encoder audio

[Maxime Ripard]

+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 <stefan at olimex.com>
> ---

> +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 <stefan at olimex.com");
> +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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200121/53541514/attachment.sig>