Re: [PATCH v14 09/10] drm/mediatek: DP audio support for MT8195

AngeloGioacchino Del Regno
2022-07-14T11:43:12Z

Il 12/07/22 13:12, Bo-Chen Chen ha scritto:
> From: Guillaume Ranquet <granquet@baylibre.com>
>
> This patch adds audio support to the DP driver for MT8195 with up to 8
> channels.
>
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 723 ++++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 2 +
> 2 files changed, 725 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 5ab646bd2bc4..fa7bb102a105 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c

..snip..

> @@ -2082,6 +2693,104 @@ static void mtk_dp_debounce_timer(struct timer_list *t)
> mtk_dp->need_debounce = true;
> }
>
> +/*
> + * HDMI audio codec callbacks
> + */
> +static int mtk_dp_audio_hw_params(struct device *dev, void *data,
> + struct hdmi_codec_daifmt *daifmt,
> + struct hdmi_codec_params *params)
> +{
> + struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
> + struct mtk_dp_audio_cfg cfg;
> +
> + if (!mtk_dp->enabled) {
> + pr_err("%s, DP is not ready!\n", __func__);

drm_err() here please.

> + return -ENODEV;
> + }
> +
> + cfg.channels = params->cea.channels;
> + cfg.sample_rate = params->sample_rate;
> + cfg.word_length_bits = 24;
> +
> + mtk_dp_audio_setup(mtk_dp, &cfg);
> +
> + return 0;
> +}
> +

..snip..

> +
> +static int mtk_dp_register_audio_driver(struct device *dev)
> +{
> + struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
> + struct hdmi_codec_pdata codec_data = {
> + .ops = &mtk_dp_audio_codec_ops,
> + .max_i2s_channels = 8,
> + .i2s = 1,
> + .data = mtk_dp,
> + };
> + struct platform_device *pdev;
> +
> + pdev = platform_device_register_data(dev, HDMI_CODEC_DRV_NAME,
> + PLATFORM_DEVID_AUTO, &codec_data,
> + sizeof(codec_data));
> + if (IS_ERR(pdev))
> + return PTR_ERR(pdev);

You're never unregistering this device, which is unacceptable.

Please add a platform device pointer in struct mtk_dp, so that this function
becomes, simply:

mtk_dp->audio_pdev = platform_device_register_data(dev,
HDMI_CODEC_DRV_NAME,
PLATFORM_DEVID_AUTO, &codec_data,
sizeof(codec_data));
return PTR_ERR_OR_ZERO(mtk_dp->audio_pdev);

> +
> + return 0;
> +}
> +
> static int mtk_dp_probe(struct platform_device *pdev)
> {
> struct mtk_dp *mtk_dp;
> @@ -2127,8 +2836,21 @@ static int mtk_dp_probe(struct platform_device *pdev)
> return dev_err_probe(dev, -EPROBE_DEFER,
> "failed to request mediatek dptx irq\n");
>
> + mutex_init(&mtk_dp->edid_lock);
> + mutex_init(&mtk_dp->eld_lock);
> + mutex_init(&mtk_dp->update_plugged_status_lock);
> +
> platform_set_drvdata(pdev, mtk_dp);
>
> + if (!mtk_dp_is_edp(mtk_dp)) {
> + ret = mtk_dp_register_audio_driver(dev);
> + if (ret) {
> + dev_err(dev, "Failed to register audio driver: %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> mtk_dp->phy_dev = platform_device_register_data(dev, "mediatek-dp-phy",
> PLATFORM_DEVID_AUTO,
> &mtk_dp->regs,
> @@ -2174,6 +2896,7 @@ static int mtk_dp_remove(struct platform_device *pdev)
>
> platform_device_unregister(mtk_dp->phy_dev);

... and unregister it here:

if (mtk_dp->audio_pdev)
platform_device_unregister(mtk_dp->audio_pdev);

> mtk_dp_video_mute(mtk_dp, true);
> + mtk_dp_audio_mute(mtk_dp, true);
> del_timer_sync(&mtk_dp->debounce_timer);
>
> pm_runtime_disable(&pdev->dev);

Regards,
Angelo