lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/2] drm: sun4i: hdmi: Add support for sun4i HDMI encoder audio
From
Date

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 <stefan@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@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.

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 <stefan@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

\
 
 \ /
  Last update: 2020-01-22 07:27    [W:0.095 / U:0.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site