mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
@ 2026-09-15 10:09 Ming Qian
  2026-09-15 14:13 ` Frank Li
  0 siblings, 1 reply; 4+ messages in thread
From: Ming Qian @ 2026-09-15 10:09 UTC (permalink / raw)
  To: Ulf Hansson, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shawn Guo, Peng Fan, Lucas Stach
  Cc: linux-pm, imx, linux-arm-kernel, linux-kernel, Zhou Peng,
	Xiahong Bao, Ming Zhou, Ming Qian

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.

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
  2026-09-15 10:09 [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains Ming Qian
@ 2026-09-15 14:13 ` Frank Li
  2026-09-16  3:06   ` Ming Qian(OSS)
  0 siblings, 1 reply; 4+ messages in thread
From: Frank Li @ 2026-09-15 14:13 UTC (permalink / raw)
  To: Ming Qian
  Cc: Ulf Hansson, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shawn Guo, Peng Fan, Lucas Stach, linux-pm, imx,
	linux-arm-kernel, linux-kernel, Zhou Peng, Xiahong Bao,
	Ming Zhou

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
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
  2026-09-15 14:13 ` Frank Li
@ 2026-09-16  3:06   ` Ming Qian(OSS)
  2026-09-16 19:31     ` Frank Li
  0 siblings, 1 reply; 4+ messages in thread
From: Ming Qian(OSS) @ 2026-09-16  3:06 UTC (permalink / raw)
  To: Frank Li
  Cc: Ulf Hansson, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shawn Guo, Peng Fan, Lucas Stach, linux-pm, imx,
	linux-arm-kernel, linux-kernel, Zhou Peng, Xiahong Bao,
	Ming Zhou

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
  2026-09-16  3:06   ` Ming Qian(OSS)
@ 2026-09-16 19:31     ` Frank Li
  0 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-09-16 19:31 UTC (permalink / raw)
  To: Ming Qian(OSS)
  Cc: Ulf Hansson, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shawn Guo, Peng Fan, Lucas Stach, linux-pm, imx,
	linux-arm-kernel, linux-kernel, Zhou Peng, Xiahong Bao,
	Ming Zhou

On Wed, Sep 16, 2026 at 11:06:02AM +0800, Ming Qian(OSS) wrote:
> 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.

good

Frank

>
>
> Thanks,
> Ming
>
> >
> > 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
> > >
> > >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-16 19:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 10:09 [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains Ming Qian
2026-09-15 14:13 ` Frank Li
2026-09-16  3:06   ` Ming Qian(OSS)
2026-09-16 19:31     ` Frank Li

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®