Re: [PATCH] drm/bridge: imx: imx8mp-hdmi-pai: enable pm runtime

[Liu Ying <victor.liu@nxp.com>]

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