[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a3a1ed17-17f7-480a-8faf-b493918956f0@gmail.com>
Date: Sat, 29 Jun 2024 12:18:15 +0200
From: Johan Jonker <jbx6244@...il.com>
To: Andy Yan <andyshrk@....com>
Cc: heiko@...ech.de, hjc@...k-chips.com, andy.yan@...k-chips.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, daniel@...ll.ch, lgirdwood@...il.com, broonie@...nel.org,
linux-sound@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7] drm/rockchip: rk3066_hdmi: add sound support
Hi Andy, thanks.
On 6/28/24 15:08, Andy Yan wrote:
>
> Hi Johan,
>
> At 2024-06-28 17:23:39, "Johan Jonker" <jbx6244@...il.com> wrote:
>> Add sound support to the RK3066 HDMI driver.
>> The HDMI TX audio source is connected to I2S_8CH.
>>
>> Signed-off-by: Zheng Yang <zhengyang@...k-chips.com>
>> Signed-off-by: Johan Jonker <jbx6244@...il.com>
>> ---
>>
>> Changed V7:
>> rebase
>> ---
>> drivers/gpu/drm/rockchip/Kconfig | 2 +
>> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 274 ++++++++++++++++++++++++-
>> 2 files changed, 275 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>> index 1bf3e2829cd0..a32ee558408c 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -102,6 +102,8 @@ config ROCKCHIP_RGB
>> config ROCKCHIP_RK3066_HDMI
>> bool "Rockchip specific extensions for RK3066 HDMI"
>> depends on DRM_ROCKCHIP
>> + select SND_SOC_HDMI_CODEC if SND_SOC
>> + select SND_SOC_ROCKCHIP_I2S if SND_SOC
>> help
>> This selects support for Rockchip SoC specific extensions
>> for the RK3066 HDMI driver. If you want to enable
>> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> index 784de990da1b..d3128b787629 100644
>> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> @@ -15,12 +15,20 @@
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>>
>> +#include <sound/hdmi-codec.h>
>> +
>> #include "rk3066_hdmi.h"
>>
>> #include "rockchip_drm_drv.h"
>>
>> #define DEFAULT_PLLA_RATE 30000000
>>
>> +struct audio_info {
>> + int channels;
>> + int sample_rate;
>> + int sample_width;
>> +};
>> +
>> struct hdmi_data_info {
>> int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
>> unsigned int enc_out_format;
>> @@ -54,9 +62,16 @@ struct rk3066_hdmi {
>>
>> unsigned int tmdsclk;
>>
>> + struct platform_device *audio_pdev;
>> + stru
>
> ......
>
>> +
>> + return ret;
>> +}
>> +
>> +static const struct hdmi_codec_ops audio_codec_ops = {
>> + .hw_params = rk3066_hdmi_audio_hw_params,
>> + .audio_shutdown = rk3066_hdmi_audio_shutdown,
>> + .mute_stream = rk3066_hdmi_audio_mute_stream,
>> + .get_eld = rk3066_hdmi_audio_get_eld,
>> + .no_capture_mute = 1,
>> +};
>> +
>> +static int rk3066_hdmi_audio_codec_init(struct rk3066_hdmi *hdmi,
>> + struct device *dev)
>> +{
>> + struct hdmi_codec_pdata codec_data = {
>> + .i2s = 1,
>> + .ops = &audio_codec_ops,
>> + .max_i2s_channels = 8,
>> + };
>> +
>> + hdmi->audio.channels = 2;
>> + hdmi->audio.sample_rate = 48000;
>> + hdmi->audio.sample_width = 16;
>> + hdmi->audio_enable = false;
>> + hdmi->audio_pdev =
>> + platform_device_register_data(dev,
>> + HDMI_CODEC_DRV_NAME,
>> + PLATFORM_DEVID_NONE,
>> + &codec_data,
>> + sizeof(codec_data));
>> +
>> + return PTR_ERR_OR_ZERO(hdmi->audio_pdev);
>> +}
>> +
>> static int
>> rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi)
>> {
>> @@ -566,6 +834,8 @@ rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi)
>>
>> drm_connector_attach_encoder(&hdmi->connector, encoder);
>>
>> + rk3066_hdmi_audio_codec_init(hdmi, dev);
>
>
> According to Documentation/driver-api/driver-model/driver.rst,
>
> It is best not to register at the bind callback:
Question for the DRM experts:
What would be the correct location/level for the rk3066_hdmi_audio_codec_init() function?
Is that at the rk3066_hdmi_encoder_enable() function?
Are there other functions/examples for sound in the DRM toolbox?
Johan
>
> .. warning::
> -EPROBE_DEFER must not be returned if probe() has already created
> child devices, even if those child devices are removed again
> in a cleanup path. If -EPROBE_DEFER is returned after a child
> device has been registered, it may result in an infinite loop of
> .probe() calls to the same driver.
>
> For example:
> vop_probe --》component_add--》rk3066_hdmi_bind--》rk3066_hdmi_audio_codec_init--》hdmi_codec_probe--》rockchip_rgb_init(DEFER when panel not ready)
>
> This may result in an infinite loop of probe
>
>
>> +
>> return 0;
>> }
>>
>> @@ -813,6 +1083,7 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
>> return 0;
>>
>> err_cleanup_hdmi:
>> + platform_device_unregister(hdmi->audio_pdev);
>> hdmi->connector.funcs->destroy(&hdmi->connector);
>> hdmi->encoder.encoder.funcs->destroy(&hdmi->encoder.encoder);
>> err_disable_i2c:
>> @@ -828,6 +1099,7 @@ static void rk3066_hdmi_unbind(struct device *dev, struct device *master,
>> {
>> struct rk3066_hdmi *hdmi = dev_get_drvdata(dev);
>>
>> + platform_device_unregister(hdmi->audio_pdev);
>> hdmi->connector.funcs->destroy(&hdmi->connector);
>> hdmi->encoder.encoder.funcs->destroy(&hdmi->encoder.encoder);
>>
>> --
>> 2.39.2
>>
Powered by blists - more mailing lists