[PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains

Tue Sep 15 20:06:02 PDT 2026

Hi Frank,

On 9/15/2026 10:13 PM, Frank Li wrote:
> 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?

Thanks for the review. Your summary is right: the power up/down reset
and clock sequences of the sibling VPU domains are not serialized
against each other.

Below is the shortened commit message. Does it look acceptable to you?

On i.MX8MP the VPU blk-ctrl exposes G1, G2 and VC8000E as three separate
genpds, each serialized only by its own genpd lock, so their power_on
and power_off callbacks can run concurrently on SMP.

The sequences are not independent: they share the VPUMIX bus domain, the
VPU_NOC and the ADB400 handshake. On power up the GPC cannot ack-verify
the ADB400 handshake - the ack only completes once blk-ctrl sets the bus
clk-en bit - so it just waits a fixed delay instead of polling hskack. A
sibling transition landing inside another domain's reset window disturbs
that shared clock and handshake timing, the victim's reset does not take
effect, and its block registers read back all zeros: the decoder times
out or the encoder fails its format check.

Serialize the blk-ctrl reset sequence with a per-blk-ctrl mutex; the
driver deliberately avoids a genpd hierarchy, so the genpd core gives no
cross-sibling serialization.

Thanks,
Ming

> 
> Frank
> 
>>
>> Fixes: a1a5f15f7f6c ("soc: imx: imx8m-blk-ctrl: add i.MX8MP VPU blk ctrl")
>> Signed-off-by: Ming Qian <ming.qian at 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
>>
>>