| 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: <CAA+D8ANQdOuEiHrOWAbqVN2oFAAop2agBzKB2_YsoQ3_SFYYNw@mail.gmail.com>
Date: Mon, 21 Jul 2025 11:41:38 +0800
From: Shengjiu Wang <shengjiu.wang@...il.com>
To: Alexander Stein <alexander.stein@...tq-group.com>
Cc: andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org,
Laurent.pinchart@...asonboard.com, jonas@...boo.se, jernej.skrabec@...il.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, simona@...ll.ch, lumag@...nel.org, dianders@...omium.org,
cristian.ciocaltea@...labora.com, luca.ceresoli@...tlin.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
victor.liu@....com, shawnguo@...nel.org, s.hauer@...gutronix.de,
kernel@...gutronix.de, festevam@...il.com, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, p.zabel@...gutronix.de, devicetree@...r.kernel.org,
l.stach@...gutronix.de, Shengjiu Wang <shengjiu.wang@....com>
Subject: Re: [PATCH 2/4] drm/bridge: imx: add driver for HDMI TX Parallel
Audio Interface
On Fri, Jul 18, 2025 at 7:51 PM Alexander Stein
<alexander.stein@...tq-group.com> wrote:
>
> Hi,
>
> thanks for sending this path.
>
> Am Freitag, 18. Juli 2025, 12:11:48 CEST schrieb Shengjiu Wang:
> > The HDMI TX Parallel Audio Interface (HTX_PAI) is a digital module that
> > acts as the bridge between the Audio Subsystem to the HDMI TX Controller.
> > This IP block is found in the HDMI subsystem of the i.MX8MP SoC.
> >
> > Data received from the audio subsystem can have an arbitrary component
> > ordering. The HTX_PAI block has integrated muxing options to select which
> > sections of the 32-bit input data word will be mapped to each IEC60958
> > field. The HTX_PAI_FIELD_CTRL register contains mux selects to
> > individually select P,C,U,V,Data, and Preamble.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> > ---
> > drivers/gpu/drm/bridge/imx/Kconfig | 7 +
> > drivers/gpu/drm/bridge/imx/Makefile | 1 +
> > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c | 134 +++++++++++++++++++
> > include/drm/bridge/dw_hdmi.h | 6 +
> > 4 files changed, 148 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> >
> > diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> > index 9a480c6abb85..d95fa84a8dcd 100644
> > --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > @@ -24,6 +24,13 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
> > Choose this to enable support for the internal HDMI encoder found
> > on the i.MX8MP SoC.
> >
> > +config DRM_IMX8MP_HDMI_PAI
> > + tristate "Freescale i.MX8MP HDMI PAI bridge support"
> > + depends on OF
> > + help
> > + Choose this to enable support for the internal HDMI TX Parallel
> > + Audio Interface found on the Freescale i.MX8MP SoC.
> > +
> > config DRM_IMX8MP_HDMI_PVI
> > tristate "Freescale i.MX8MP HDMI PVI bridge support"
> > depends on OF
> > diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> > index dd5d48584806..8d01fda25451 100644
> > --- a/drivers/gpu/drm/bridge/imx/Makefile
> > +++ b/drivers/gpu/drm/bridge/imx/Makefile
> > @@ -1,6 +1,7 @@
> > obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
> > obj-$(CONFIG_DRM_IMX_LEGACY_BRIDGE) += imx-legacy-bridge.o
> > obj-$(CONFIG_DRM_IMX8MP_DW_HDMI_BRIDGE) += imx8mp-hdmi-tx.o
> > +obj-$(CONFIG_DRM_IMX8MP_HDMI_PAI) += imx8mp-hdmi-pai.o
> > obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
> > obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
> > obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> > new file mode 100644
> > index 000000000000..f09ee2622e57
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2025 NXP
> > + */
> > +
> > +#include <drm/bridge/dw_hdmi.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define HTX_PAI_CTRL 0x00
> > +#define HTX_PAI_CTRL_EXT 0x04
> > +#define HTX_PAI_FIELD_CTRL 0x08
> > +#define HTX_PAI_STAT 0x0c
> > +#define HTX_PAI_IRQ_NOMASK 0x10
> > +#define HTX_PAI_IRQ_MASKED 0x14
> > +#define HTX_PAI_IRQ_MASK 0x18
> > +
> > +#define CTRL_ENABLE BIT(0)
> > +
> > +#define CTRL_EXT_WTMK_HIGH_MASK GENMASK(31, 24)
> > +#define CTRL_EXT_WTMK_HIGH (0x3 << 24)
> > +#define CTRL_EXT_WTMK_LOW_MASK GENMASK(23, 16)
> > +#define CTRL_EXT_WTMK_LOW (0x3 << 16)
> > +#define CTRL_EXT_NUM_CH_MASK GENMASK(10, 8)
> > +#define CTRL_EXT_NUM_CH_SHIFT 8
> > +
> > +#define FIELD_CTRL_B_FILT BIT(31)
> > +#define FIELD_CTRL_PARITY_EN BIT(30)
> > +#define FIELD_CTRL_END_SEL BIT(29)
> > +#define FIELD_CTRL_PRE_SEL GENMASK(28, 24)
> > +#define FIELD_CTRL_PRE_SEL_SHIFT 24
> > +#define FIELD_CTRL_D_SEL GENMASK(23, 20)
> > +#define FIELD_CTRL_D_SEL_SHIFT 20
> > +#define FIELD_CTRL_V_SEL GENMASK(19, 15)
> > +#define FIELD_CTRL_V_SEL_SHIFT 15
> > +#define FIELD_CTRL_U_SEL GENMASK(14, 10)
> > +#define FIELD_CTRL_U_SEL_SHIFT 10
> > +#define FIELD_CTRL_C_SEL GENMASK(9, 5)
> > +#define FIELD_CTRL_C_SEL_SHIFT 5
> > +#define FIELD_CTRL_P_SEL GENMASK(4, 0)
> > +#define FIELD_CTRL_P_SEL_SHIFT 0
> > +
> > +struct imx8mp_hdmi_pai {
> > + struct device *dev;
> > + void __iomem *base;
> > +};
> > +
> > +static void imx8mp_hdmi_pai_enable(struct dw_hdmi *dw_hdmi, int channel,
> > + int width, int rate, int non_pcm)
> > +{
> > + const struct dw_hdmi_plat_data *pdata = dw_hdmi_to_plat_data(dw_hdmi);
> > + struct imx8mp_hdmi_pai *hdmi_pai = (struct imx8mp_hdmi_pai *)pdata->priv_audio;
> > + int val;
> > +
> > + /* PAI set */
> > + val = CTRL_EXT_WTMK_HIGH | CTRL_EXT_WTMK_LOW;
> > + val |= ((channel - 1) << CTRL_EXT_NUM_CH_SHIFT);
> > + writel(val, hdmi_pai->base + HTX_PAI_CTRL_EXT);
> > +
> > + /* IEC60958 format */
> > + val = 31 << FIELD_CTRL_P_SEL_SHIFT;
> > + val |= 30 << FIELD_CTRL_C_SEL_SHIFT;
> > + val |= 29 << FIELD_CTRL_U_SEL_SHIFT;
> > + val |= 28 << FIELD_CTRL_V_SEL_SHIFT;
> > + val |= 4 << FIELD_CTRL_D_SEL_SHIFT;
> > + val |= 0 << FIELD_CTRL_PRE_SEL_SHIFT;
>
> You can use FIELD_PREP_CONST() for these shifts as the GENMASK are
> already available.
Ok.
> But where do these numbers come from? I can see that downstream kernel
> sets these bits depending on audio config being passed.
These numbers are defined in standard IEC958 spec.
The implementation in downstream kernel may not good enough, which can't
distinguish between the raw PCM and IEC958 PCM. Even raw PCM can work,
but in RM, the supported format is IEC958.
So later we need to update the aud2htx driver to only support IEC958 format.
Make the alignment between PAI driver and aud2htx driver.
>
> > +
> > + writel(val, hdmi_pai->base + HTX_PAI_FIELD_CTRL);
> > + /* PAI start running */
> > + writel(CTRL_ENABLE, hdmi_pai->base + HTX_PAI_CTRL);
> > +}
> > +
> > +static void imx8mp_hdmi_pai_disable(struct dw_hdmi *dw_hdmi)
> > +{
> > + const struct dw_hdmi_plat_data *pdata = dw_hdmi_to_plat_data(dw_hdmi);
> > + struct imx8mp_hdmi_pai *hdmi_pai = (struct imx8mp_hdmi_pai *)pdata->priv_audio;
> > +
> > + /* Stop PAI */
> > + writel(0, hdmi_pai->base + HTX_PAI_CTRL);
> > +}
> > +
> > +static int imx8mp_hdmi_pai_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct dw_hdmi_plat_data *plat_data;
> > + struct imx8mp_hdmi_pai *hdmi_pai;
> > + struct device_node *remote;
> > + struct platform_device *hdmi_tx;
> > + struct resource *res;
> > +
> > + hdmi_pai = devm_kzalloc(dev, sizeof(*hdmi_pai), GFP_KERNEL);
> > + if (!hdmi_pai)
> > + return -ENOMEM;
> > +
> > + hdmi_pai->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> > + if (IS_ERR(hdmi_pai->base))
> > + return PTR_ERR(hdmi_pai->base);
> > +
> > + hdmi_pai->dev = dev;
> > +
> > + remote = of_graph_get_remote_node(pdev->dev.of_node, 0, -1);
> > + if (!remote)
> > + return -EINVAL;
> > +
> > + hdmi_tx = of_find_device_by_node(remote);
> > + if (!hdmi_tx)
> > + return -EINVAL;
> > +
> > + plat_data = platform_get_drvdata(hdmi_tx);
>
> How do you ensure hdmi_tx has been probed so that drvdata is set?
> Also hdmi_tx needs to be dropped.
Good catch. need to update to
hdmi_tx = of_find_device_by_node(remote);
of_node_put(remote);
if (!hdmi_tx)
return -ENODEV;
plat_data = platform_get_drvdata(hdmi_tx);
put_device(&hdmi_tx->dev);
if (!plat_data)
return -EPROBE_DEFER;
>
> > + plat_data->enable_audio = imx8mp_hdmi_pai_enable;
> > + plat_data->disable_audio = imx8mp_hdmi_pai_disable;
> > + plat_data->priv_audio = hdmi_pai;
>
> How do you connect this device to aud2htx?
aud2htx -> hdmi_codec -> dw-hdmi-gp-audio
hdmi_codec is sound/soc/codecs/hdmi-codec.c
gp-audio will call this plat_data->enable(disable)_audio().
So need to register the plat_data->enable(disable)_audio in this driver.
Best regards
Shengjiu Wang
>
> Best regards,
> Alexander
>
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id imx8mp_hdmi_pai_of_table[] = {
> > + { .compatible = "fsl,imx8mp-hdmi-pai" },
> > + { /* Sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8mp_hdmi_pai_of_table);
> > +
> > +static struct platform_driver imx8mp_hdmi_pai_platform_driver = {
> > + .probe = imx8mp_hdmi_pai_probe,
> > + .driver = {
> > + .name = "imx8mp-hdmi-pai",
> > + .of_match_table = imx8mp_hdmi_pai_of_table,
> > + },
> > +};
> > +module_platform_driver(imx8mp_hdmi_pai_platform_driver);
> > +
> > +MODULE_DESCRIPTION("i.MX8MP HDMI PAI driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index a56a3519a22a..9ca70ce80cc5 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -143,6 +143,12 @@ struct dw_hdmi_plat_data {
> > const struct drm_display_info *info,
> > const struct drm_display_mode *mode);
> >
> > + /*
> > + * priv_audio is specially used for additional audio device to get
> > + * driver data through this dw_hdmi_plat_data.
> > + */
> > + void *priv_audio;
> > +
> > /* Platform-specific audio enable/disable (optional) */
> > void (*enable_audio)(struct dw_hdmi *hdmi, int channel,
> > int width, int rate, int non_pcm);
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>
Powered by blists - more mailing lists