| 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
| ||
|
Message-ID: <mubznqagapsm2n3eym7664u2x6mgjxa2phsalk5w647uecvson@7q3okwl3mgu4>
Date: Tue, 21 Jan 2025 12:15:31 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Hermes.wu@....com.tw
Cc: Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>,
Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, treapking@...omium.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, Pet.Weng@....com.tw,
Kenneth.Hung@....com.tw
Subject: Re: [PATCH] drm/bridge: it6505: support hdmi_codec_ops for audio
stream setup
On Tue, Jan 21, 2025 at 04:59:22PM +0800, Hermes Wu via B4 Relay wrote:
> From: Hermes Wu <Hermes.wu@....com.tw>
>
> IT6505 supports audio form I2S to DP audio data sub stream
>
> Support audio codec operation include
> hw_params, audio_startup, audio_shutdown, hook_plugged_cb.
The DRM framework recently got generic HDMI codec framework
(display/drm_hdmi_audio_helper.c), drm_bridge callbacks to implement
HDMI codec functions and glue code in drm_bridge_connector.c.
Please consider using it instead of manually implementing all the glue
on your own. The drm_bridge_connector code is limited to the HDMI case,
but it can be extended to support DP too.
> In order to prevent pop noise from sink devise, delay audio by
> after I2S signal is enable by source.
>
> Signed-off-by: Hermes Wu <Hermes.wu@....com.tw>
> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 67 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 88ef76a37fe6accacdd343839ff2569b31b18ceb..9dc58d307dae360ffab5df15e8fe8420d084c764 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -3095,18 +3095,39 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
> return 0;
> }
>
> -static void __maybe_unused it6505_audio_shutdown(struct device *dev, void *data)
> +static void it6505_audio_shutdown(struct device *dev, void *data)
> {
> struct it6505 *it6505 = dev_get_drvdata(dev);
>
> + cancel_delayed_work_sync(&it6505->delayed_audio);
> +
> if (it6505->powered)
> it6505_disable_audio(it6505);
> }
>
> -static int __maybe_unused it6505_audio_hook_plugged_cb(struct device *dev,
> - void *data,
> - hdmi_codec_plugged_cb fn,
> - struct device *codec_dev)
> +static int it6505_audio_hw_params(struct device *dev,
> + void *data,
> + struct hdmi_codec_daifmt *daifmt,
> + struct hdmi_codec_params *params)
> +{
> + struct it6505 *it6505 = dev_get_drvdata(dev);
> +
> + return it6505_audio_setup_hw_params(it6505, params);
> +}
> +
> +static int it6505_audio_startup(struct device *dev, void *data)
> +{
> + struct it6505 *it6505 = dev_get_drvdata(dev);
> +
> + queue_delayed_work(system_wq, &it6505->delayed_audio,
> + msecs_to_jiffies(180));
Where does 180 msec delay come from? It sounds like some kind of
platform issue rather than the IT6505 issue. Can you change the order of
the operations in the platform code?
> + return 0;
> +}
> +
> +static int it6505_audio_hook_plugged_cb(struct device *dev,
> + void *data,
> + hdmi_codec_plugged_cb fn,
> + struct device *codec_dev)
> {
> struct it6505 *it6505 = data;
>
> @@ -3117,6 +3138,36 @@ static int __maybe_unused it6505_audio_hook_plugged_cb(struct device *dev,
> return 0;
> }
>
> +static const struct hdmi_codec_ops it6505_audio_codec_ops = {
> + .hw_params = it6505_audio_hw_params,
> + .audio_startup = it6505_audio_startup,
> + .audio_shutdown = it6505_audio_shutdown,
> + .hook_plugged_cb = it6505_audio_hook_plugged_cb,
> +};
> +
> +static int it6505_register_audio_driver(struct device *dev)
> +{
> + struct it6505 *it6505 = dev_get_drvdata(dev);
> + struct hdmi_codec_pdata codec_data = {
> + .ops = &it6505_audio_codec_ops,
> + .max_i2s_channels = 8,
> + .i2s = 1,
> + .data = it6505,
> + };
> + 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);
> +
> + INIT_DELAYED_WORK(&it6505->delayed_audio, it6505_delayed_audio);
> + DRM_DEV_DEBUG_DRIVER(dev, "bound to %s", HDMI_CODEC_DRV_NAME);
it's not bound, the message is useless.
> +
> + return 0;
> +}
> +
> static inline struct it6505 *bridge_to_it6505(struct drm_bridge *bridge)
> {
> return container_of(bridge, struct it6505, bridge);
> @@ -3677,6 +3728,12 @@ static int it6505_i2c_probe(struct i2c_client *client)
> return err;
> }
>
> + err = it6505_register_audio_driver(dev);
There is no code to unregister the platform device.
> + if (err < 0) {
> + dev_err(dev, "Failed to register audio driver: %d", err);
> + return err;
> + }
> +
> INIT_WORK(&it6505->link_works, it6505_link_training_work);
> INIT_WORK(&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);
> INIT_DELAYED_WORK(&it6505->hdcp_work, it6505_hdcp_work);
>
> ---
> base-commit: fe003bcb69f7bff9ff2b30b659b004dbafe52907
> change-id: 20250114-add-audio-codec-8c9d47062a6c
>
> Best regards,
> --
> Hermes Wu <Hermes.wu@....com.tw>
>
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists