[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cec25a41-cc6a-4b03-833e-5c7da5727e85@nxp.com>
Date: Fri, 30 Jan 2026 14:49:38 +0800
From: Liu Ying <victor.liu@....com>
To: Shengjiu Wang <shengjiu.wang@....com>, 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,
shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, dri-devel@...ts.freedesktop.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: imx: imx8mp-hdmi-pai: enable pm runtime
Hi Shengjiu,
On Fri, Jan 30, 2026 at 01:55:08PM +0800, Shengjiu Wang wrote:
> There is an audio channel shift issue with multi channel case, first run
> the channel order is correct, but second run the channel order is shifted.
Nit: I'd rephrase "There is an audio channel shift issue with multi channel
case - the channel order is correct for the first run, but the channel order
is shifted for the second run.".
> The recovery method is to reset the pai interface.
s/pai/PAI/
Can the channel order shift issue be fully fixed instead of implementing
a "recovery method"? I presume that this patch implements a recovery
mechanism. If it can't, any chip ERRATA?
>
> The reset can be handled by pm runtime, so add the pm runtime function.
Nit: s/pm/PM/
s/add the pm runtime function/enable PM runtime/
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
> drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Subject prefix should be "drm/bridge: imx8mp-hdmi-pai:".
Refer to what previous commits for i.MX bridge drivers use.
>
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> index 8d13a35b206a..1afc729da9b9 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pai.c
> @@ -8,6 +8,7 @@
> #include <linux/module.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <drm/bridge/dw_hdmi.h>
> #include <sound/asoundef.h>
> @@ -33,6 +34,7 @@
>
> struct imx8mp_hdmi_pai {
> struct regmap *regmap;
> + struct device *dev;
> };
>
> static void imx8mp_hdmi_pai_enable(struct dw_hdmi *dw_hdmi, int channel,
> @@ -43,6 +45,9 @@ static void imx8mp_hdmi_pai_enable(struct dw_hdmi *dw_hdmi, int channel,
> struct imx8mp_hdmi_pai *hdmi_pai = pdata->priv_audio;
> int val;
>
> + if (pm_runtime_get_sync(hdmi_pai->dev) < 0)
Use pm_runtime_resume_and_get instead.
-8<-
`int pm_runtime_get_sync(struct device *dev);`
- increment the device's usage counter, run pm_runtime_resume(dev) and
return its result;
note that it does not drop the device's usage counter on errors, so
consider using pm_runtime_resume_and_get() instead of it, especially
if its return value is checked by the caller, as this is likely to
result in cleaner code.
-8<-
> + return;
> +
> /* PAI set control extended */
> val = WTMK_HIGH(3) | WTMK_LOW(3);
> val |= NUM_CH(channel);
> @@ -85,6 +90,8 @@ static void imx8mp_hdmi_pai_disable(struct dw_hdmi *dw_hdmi)
>
> /* Stop PAI */
> regmap_write(hdmi_pai->regmap, HTX_PAI_CTRL, 0);
> +
> + pm_runtime_put_sync(hdmi_pai->dev);
> }
>
> static const struct regmap_config imx8mp_hdmi_pai_regmap_config = {
> @@ -101,6 +108,7 @@ static int imx8mp_hdmi_pai_bind(struct device *dev, struct device *master, void
> struct imx8mp_hdmi_pai *hdmi_pai;
> struct resource *res;
> void __iomem *base;
> + int ret;
>
> hdmi_pai = devm_kzalloc(dev, sizeof(*hdmi_pai), GFP_KERNEL);
> if (!hdmi_pai)
> @@ -121,6 +129,13 @@ static int imx8mp_hdmi_pai_bind(struct device *dev, struct device *master, void
> plat_data->disable_audio = imx8mp_hdmi_pai_disable;
> plat_data->priv_audio = hdmi_pai;
>
> + hdmi_pai->dev = dev;
> + ret = devm_pm_runtime_enable(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable pm runtime: %d\n", ret);
s/pm/PM/
> + return ret;
> + }
> +
> return 0;
> }
>
--
Regards,
Liu Ying
Powered by blists - more mailing lists