| 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: <3f780b9c-d670-4b67-94f9-e5b59bcee7da@nxp.com>
Date: Mon, 11 Aug 2025 10:15:48 +0800
From: Liu Ying <victor.liu@....com>
To: Frank Li <Frank.li@....com>, Shengjiu Wang <shengjiu.wang@....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, 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@...il.com, perex@...ex.cz,
tiwai@...e.com, linux-sound@...r.kernel.org
Subject: Re: [PATCH v4 5/7] drm/bridge: imx: add driver for HDMI TX Parallel
Audio Interface
On 08/08/2025, Frank Li wrote:
> On Fri, Aug 08, 2025 at 04:06:15PM +0800, Shengjiu Wang wrote:
>> 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.
>>
>> Use component helper so that imx8mp-hdmi-tx will be aggregate driver,
>> imx8mp-hdmi-pai will be component driver, then imx8mp-hdmi-pai can use
>> bind() ops to get the plat_data from imx8mp-hdmi-tx device.
>>
>> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
>> ---
>> drivers/gpu/drm/bridge/imx/Kconfig | 11 ++
>> drivers/gpu/drm/bridge/imx/Makefile | 1 +
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c | 158 +++++++++++++++++++
>> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 63 +++++++-
>> include/drm/bridge/dw_hdmi.h | 6 +
>> 5 files changed, 234 insertions(+), 5 deletions(-)
>> 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..b9028a5e5a06 100644
>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>> @@ -18,12 +18,23 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>> depends on OF
>> depends on COMMON_CLK
>> select DRM_DW_HDMI
>> + imply DRM_IMX8MP_HDMI_PAI
>> imply DRM_IMX8MP_HDMI_PVI
>> imply PHY_FSL_SAMSUNG_HDMI_PHY
>> help
>> 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
>> + select DRM_DW_HDMI
>> + select REGMAP
>> + select REGMAP_MMIO
>> + 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..8d13a35b206a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
>> @@ -0,0 +1,158 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2025 NXP
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/component.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <drm/bridge/dw_hdmi.h>
>> +#include <sound/asoundef.h>
>> +
>> +#define HTX_PAI_CTRL 0x00
>> +#define ENABLE BIT(0)
>> +
>> +#define HTX_PAI_CTRL_EXT 0x04
>> +#define WTMK_HIGH_MASK GENMASK(31, 24)
>> +#define WTMK_LOW_MASK GENMASK(23, 16)
>> +#define NUM_CH_MASK GENMASK(10, 8)
>> +#define WTMK_HIGH(n) FIELD_PREP(WTMK_HIGH_MASK, (n))
>> +#define WTMK_LOW(n) FIELD_PREP(WTMK_LOW_MASK, (n))
>> +#define NUM_CH(n) FIELD_PREP(NUM_CH_MASK, (n) - 1)
>> +
>> +#define HTX_PAI_FIELD_CTRL 0x08
>> +#define PRE_SEL GENMASK(28, 24)
>> +#define D_SEL GENMASK(23, 20)
>> +#define V_SEL GENMASK(19, 15)
>> +#define U_SEL GENMASK(14, 10)
>> +#define C_SEL GENMASK(9, 5)
>> +#define P_SEL GENMASK(4, 0)
>> +
>> +struct imx8mp_hdmi_pai {
>> + struct regmap *regmap;
>> +};
>> +
>> +static void imx8mp_hdmi_pai_enable(struct dw_hdmi *dw_hdmi, int channel,
>> + int width, int rate, int non_pcm,
>> + int iec958)
>> +{
>> + const struct dw_hdmi_plat_data *pdata = dw_hdmi_to_plat_data(dw_hdmi);
>> + struct imx8mp_hdmi_pai *hdmi_pai = pdata->priv_audio;
>> + int val;
>> +
>> + /* PAI set control extended */
>> + val = WTMK_HIGH(3) | WTMK_LOW(3);
>> + val |= NUM_CH(channel);
>> + regmap_write(hdmi_pai->regmap, HTX_PAI_CTRL_EXT, val);
>> +
>> + /* IEC60958 format */
>> + if (iec958) {
>> + val = FIELD_PREP_CONST(P_SEL,
>> + __bf_shf(IEC958_SUBFRAME_PARITY));
>> + val |= FIELD_PREP_CONST(C_SEL,
>> + __bf_shf(IEC958_SUBFRAME_CHANNEL_STATUS));
>> + val |= FIELD_PREP_CONST(U_SEL,
>> + __bf_shf(IEC958_SUBFRAME_USER_DATA));
>> + val |= FIELD_PREP_CONST(V_SEL,
>> + __bf_shf(IEC958_SUBFRAME_VALIDITY));
>> + val |= FIELD_PREP_CONST(D_SEL,
>> + __bf_shf(IEC958_SUBFRAME_SAMPLE_24_MASK));
>> + val |= FIELD_PREP_CONST(PRE_SEL,
>> + __bf_shf(IEC958_SUBFRAME_PREAMBLE_MASK));
>> + } else {
>> + /*
>> + * The allowed PCM widths are 24bit and 32bit, as they are supported
>> + * by aud2htx module.
>> + * for 24bit, D_SEL = 0, select all the bits.
>> + * for 32bit, D_SEL = 8, select 24bit in MSB.
>> + */
>> + val = FIELD_PREP(D_SEL, width - 24);
>> + }
>> +
>> + regmap_write(hdmi_pai->regmap, HTX_PAI_FIELD_CTRL, val);
>> +
>> + /* PAI start running */
>> + regmap_write(hdmi_pai->regmap, HTX_PAI_CTRL, ENABLE);
>> +}
>> +
>> +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 = pdata->priv_audio;
>> +
>> + /* Stop PAI */
>> + regmap_write(hdmi_pai->regmap, HTX_PAI_CTRL, 0);
>> +}
>> +
>> +static const struct regmap_config imx8mp_hdmi_pai_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = HTX_PAI_FIELD_CTRL,
>> +};
>> +
>> +static int imx8mp_hdmi_pai_bind(struct device *dev, struct device *master, void *data)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct dw_hdmi_plat_data *plat_data = data;
>> + struct imx8mp_hdmi_pai *hdmi_pai;
>> + struct resource *res;
>> + void __iomem *base;
>> +
>> + hdmi_pai = devm_kzalloc(dev, sizeof(*hdmi_pai), GFP_KERNEL);
>> + if (!hdmi_pai)
>> + return -ENOMEM;
>
> I am not sure how bind/unbind work here. you use devm_ func here? does
> system auto free it at unbind()?
Component helper supports freeing resources allocated by device managed APIs.
The allocations are commonly used by bind callbacks in various drivers.
>
> generally, free() happen and driver's remove if use devm_ func
>
> Frank
>> +
>> + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + hdmi_pai->regmap = devm_regmap_init_mmio_clk(dev, "apb", base,
>> + &imx8mp_hdmi_pai_regmap_config);
>> + if (IS_ERR(hdmi_pai->regmap)) {
>> + dev_err(dev, "regmap init failed\n");
>> + return PTR_ERR(hdmi_pai->regmap);
>> + }
>> +
>> + plat_data->enable_audio = imx8mp_hdmi_pai_enable;
>> + plat_data->disable_audio = imx8mp_hdmi_pai_disable;
>> + plat_data->priv_audio = hdmi_pai;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct component_ops imx8mp_hdmi_pai_ops = {
>> + .bind = imx8mp_hdmi_pai_bind,
>> +};
>> +
>> +static int imx8mp_hdmi_pai_probe(struct platform_device *pdev)
>> +{
>> + return component_add(&pdev->dev, &imx8mp_hdmi_pai_ops);
>> +}
>> +
>> +static void imx8mp_hdmi_pai_remove(struct platform_device *pdev)
>> +{
>> + component_del(&pdev->dev, &imx8mp_hdmi_pai_ops);
>> +}
>> +
>> +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,
>> + .remove = imx8mp_hdmi_pai_remove,
>> + .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/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> index 1e7a789ec289..c68896392a2d 100644
>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>> @@ -5,11 +5,13 @@
>> */
>>
>> #include <linux/clk.h>
>> +#include <linux/component.h>
>> #include <linux/mod_devicetable.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <drm/bridge/dw_hdmi.h>
>> #include <drm/drm_modes.h>
>> +#include <drm/drm_of.h>
>>
>> struct imx8mp_hdmi {
>> struct dw_hdmi_plat_data plat_data;
>> @@ -79,10 +81,43 @@ static const struct dw_hdmi_phy_ops imx8mp_hdmi_phy_ops = {
>> .update_hpd = dw_hdmi_phy_update_hpd,
>> };
>>
>> +static int imx8mp_dw_hdmi_bind(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct imx8mp_hdmi *hdmi = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + ret = component_bind_all(dev, &hdmi->plat_data);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "component_bind_all failed!\n");
>> +
>> + hdmi->dw_hdmi = dw_hdmi_probe(pdev, &hdmi->plat_data);
>> + if (IS_ERR(hdmi->dw_hdmi))
>> + return PTR_ERR(hdmi->dw_hdmi);
>> +
>> + return 0;
>> +}
>> +
>> +static void imx8mp_dw_hdmi_unbind(struct device *dev)
>> +{
>> + struct imx8mp_hdmi *hdmi = dev_get_drvdata(dev);
>> +
>> + dw_hdmi_remove(hdmi->dw_hdmi);
>> +
>> + component_unbind_all(dev, &hdmi->plat_data);
>> +}
>> +
>> +static const struct component_master_ops imx8mp_dw_hdmi_ops = {
>> + .bind = imx8mp_dw_hdmi_bind,
>> + .unbind = imx8mp_dw_hdmi_unbind,
>> +};
>> +
>> static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct dw_hdmi_plat_data *plat_data;
>> + struct component_match *match = NULL;
>> + struct device_node *remote;
>> struct imx8mp_hdmi *hdmi;
>>
>> hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
>> @@ -102,20 +137,38 @@ static int imx8mp_dw_hdmi_probe(struct platform_device *pdev)
>> plat_data->priv_data = hdmi;
>> plat_data->phy_force_vendor = true;
>>
>> - hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>> - if (IS_ERR(hdmi->dw_hdmi))
>> - return PTR_ERR(hdmi->dw_hdmi);
>> -
>> platform_set_drvdata(pdev, hdmi);
>>
>> + /* port@2 is for hdmi_pai device */
>> + remote = of_graph_get_remote_node(pdev->dev.of_node, 2, 0);
>> + if (!remote) {
>> + hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>> + if (IS_ERR(hdmi->dw_hdmi))
>> + return PTR_ERR(hdmi->dw_hdmi);
>> + } else {
>> + drm_of_component_match_add(dev, &match, component_compare_of, remote);
>> +
>> + of_node_put(remote);
>> +
>> + return component_master_add_with_match(dev, &imx8mp_dw_hdmi_ops, match);
>> + }
>> +
>> return 0;
>> }
>>
>> static void imx8mp_dw_hdmi_remove(struct platform_device *pdev)
>> {
>> struct imx8mp_hdmi *hdmi = platform_get_drvdata(pdev);
>> + struct device_node *remote;
>>
>> - dw_hdmi_remove(hdmi->dw_hdmi);
>> + remote = of_graph_get_remote_node(pdev->dev.of_node, 2, 0);
>> + if (remote) {
>> + of_node_put(remote);
>> +
>> + component_master_del(&pdev->dev, &imx8mp_dw_hdmi_ops);
>> + } else {
>> + dw_hdmi_remove(hdmi->dw_hdmi);
>> + }
>> }
>>
>> static int imx8mp_dw_hdmi_pm_suspend(struct device *dev)
>> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
>> index 095cdd9b7424..336f062e1f9d 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, int iec958);
>> --
>> 2.34.1
>>
--
Regards,
Liu Ying
Powered by blists - more mailing lists