linux-kernel - Re:[PATCH v7] drm/rockchip: rk3066

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7c53f7d.bb08.1905ef690ef.Coremail.andyshrk@163.com>
Date: Fri, 28 Jun 2024 21:08:55 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Johan Jonker" <jbx6244@...il.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 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：

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