From: Frank Li <Frank.li@oss.nxp.com>
To: Ming Qian <ming.qian@oss.nxp.com>
Cc: Ulf Hansson <ulfh@kernel.org>, Frank Li <Frank.Li@nxp.com>,
Sascha Hauer <s.hauer@pengutronix.de>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Fabio Estevam <festevam@gmail.com>,
Shawn Guo <shawnguo@kernel.org>, Peng Fan <peng.fan@nxp.com>,
Lucas Stach <l.stach@pengutronix.de>,
linux-pm@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Zhou Peng <eagle.zhou@nxp.com>,
Xiahong Bao <xiahong.bao@nxp.com>, Ming Zhou <ming.zhou@nxp.com>
Subject: Re: [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
Date: Tue, 15 Sep 2026 09:13:14 -0500 [thread overview]
Message-ID: <aqlSerHyjCB-03IR@SMW015318> (raw)
In-Reply-To: <20260915-imx8mp-blk-ctrl-v1-1-b3b4e6e7e676@oss.nxp.com>
On Tue, Sep 15, 2026 at 07:09:43PM +0900, Ming Qian wrote:
> On i.MX8MP, running the VC8000E encoder and the G1/G2 decoders
> concurrently rarely and non-deterministically leaves a VPU stuck in
> reset: its block registers read back all zeros. A decoder then times out
> (G1/G2 reset failed) or the encoder fails its format check reading a
> read-only capability register (VC8000E reset failed). Raising the
> runtime-PM autosuspend delay hides it, which points at the blk-ctrl
> power on/off path.
>
> The VPU blk-ctrl exposes G1, G2 and VC8000E as three separate genpds
> whose power_on/power_off are serialized only by the per-genpd lock, so on
> SMP the callbacks of different domains can run concurrently. Each domain
> only manipulates its own bit in the shared BLK_SFT_RSTN/BLK_CLK_EN
> registers, so this is not a matter of siblings corrupting each other's
> register bits.
>
> However the sibling power transitions still interact through the shared
> VPUMIX resources: the VPUMIX bus power domain, the VPU_NOC and the ADB400
> handshake. power_on asserts a domain's reset, enables its clock and
> releases the reset after a short udelay, relying on the reset propagating
> through that shared path. The GPC does not ack-verify the ADB400
> handshake on power-up (it only delays), and per ERR050531 the VPU_NOC
> handshake is timing sensitive during VC8000E/VPUMIX power up/down
> cycling. So when a sibling's power_on/power_off runs while another domain
> is inside its reset window, it disturbs the shared VPU_NOC/ADB/AXI clock
> timing and the victim's reset fails to take effect, leaving its block in
> reset with registers reading zero.
>
> This is a blk-ctrl defect: the VPU power domains' power up/down sequences
> must not interleave, yet the driver deliberately avoids a genpd
> parent/child hierarchy (to meet its sequencing requirements) and so gets
> no cross-sibling serialization from the genpd core.
>
> Add a per-blk-ctrl mutex around the blk-ctrl register and reset sequence
> and the synchronous power-up path, so a sibling domain cannot run its
> sequence while another domain is inside its reset window. The GPC
> power-down is queued by pm_runtime_put() and still completes outside the
> lock; serializing the reset sequences is what fixes the observed failure.
> The bus domain's GENPD_NOTIFY_ON notifier runs in the same call stack
> while the lock is held and must not take it.
Thanks you for detail descript problem, basically it is power up/down
reset, clock have not serialized.
Can you help summery to cut message shorter end emphase most important
part?
Frank
>
> Fixes: a1a5f15f7f6c ("soc: imx: imx8m-blk-ctrl: add i.MX8MP VPU blk ctrl")
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
> Reproduced on i.MX8MP with an Android 6.18 kernel: running an H.264
> decode and an H.264 encode concurrently hits the failure within one to
> two hours. A VPU comes up stuck in reset, its block registers read back
> all zeros, and the decoder times out or the encoder fails its format
> check.
>
> With this patch applied the same test ran overnight, over 14 hours,
> without a single occurrence.
> ---
> drivers/pmdomain/imx/imx8m-blk-ctrl.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 479789009c7f..f8105e87ea3c 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -15,6 +15,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/clk.h>
> +#include <linux/mutex.h>
>
> #include <dt-bindings/power/imx8mm-power.h>
> #include <dt-bindings/power/imx8mn-power.h>
> @@ -34,6 +35,12 @@ struct imx8m_blk_ctrl {
> struct regmap *regmap;
> struct imx8m_blk_ctrl_domain *domains;
> struct genpd_onecell_data onecell_data;
> + /*
> + * Serializes the blk-ctrl reset/clock sequence across sibling domains;
> + * their transitions interact through the shared VPUMIX bus domain,
> + * VPU_NOC and the not-ack-verified ADB400 handshake (ERR050531).
> + */
> + struct mutex power_lock;
> };
>
> struct imx8m_blk_ctrl_domain_data {
> @@ -98,6 +105,8 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> struct imx8m_blk_ctrl *bc = domain->bc;
> int ret;
>
> + guard(mutex)(&bc->power_lock);
> +
> /* make sure bus domain is awake */
> ret = pm_runtime_get_sync(bc->bus_power_dev);
> if (ret < 0) {
> @@ -164,6 +173,8 @@ static int imx8m_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> struct imx8m_blk_ctrl *bc = domain->bc;
>
> + guard(mutex)(&bc->power_lock);
> +
> /* put devices into reset and disable clocks */
> if (data->mipi_phy_rst_mask)
> regmap_clear_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);
> @@ -202,6 +213,10 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>
> bc->dev = dev;
>
> + ret = devm_mutex_init(dev, &bc->power_lock);
> + if (ret)
> + return ret;
> +
> bc_data = of_device_get_match_data(dev);
>
> base = devm_platform_ioremap_resource(pdev, 0);
>
> ---
> base-commit: 6e30287eaf3e41b86dfb86df3b811526693d74b4
> change-id: 20260911-imx8mp-blk-ctrl-c46f26783073
>
>
next prev parent reply other threads:[~2026-09-15 14:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-15 10:09 Ming Qian
2026-09-15 14:13 ` Frank Li [this message]
2026-09-16 3:06 ` Ming Qian(OSS)
2026-09-16 19:31 ` Frank Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqlSerHyjCB-03IR@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@nxp.com \
--cc=eagle.zhou@nxp.com \
--cc=festevam@gmail.com \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--cc=l.stach@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=ming.qian@oss.nxp.com \
--cc=ming.zhou@nxp.com \
--cc=peng.fan@nxp.com \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=ulfh@kernel.org \
--cc=xiahong.bao@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®