1. Patchew
  2. linux
  3. View series

[PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off

Dikshita Agarwal posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
drivers/media/platform/qcom/iris/iris_vpu3x.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
[PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Dikshita Agarwal 1 month, 3 weeks ago 
Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
reading the LPI status register. Introduce a handshake loop to ensure
MNoC enters low power mode reliably during VPU3 hardware power-off with
timeout handling.

Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
---
 drivers/media/platform/qcom/iris/iris_vpu3x.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index 9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a..c2e6af575cbe4b3e3f2a019b24eecf3a5d469566 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -110,6 +110,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
 static void iris_vpu33_power_off_hardware(struct iris_core *core)
 {
 	u32 reg_val = 0, value, i;
+	u32 count = 0;
 	int ret;
 
 	if (iris_vpu3x_hw_power_collapsed(core))
@@ -128,13 +129,31 @@ static void iris_vpu33_power_off_hardware(struct iris_core *core)
 			goto disable_power;
 	}
 
+	/* set MNoC to low power */
+	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+
+	value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
+
+	while (!(value & BIT(0)) && (value & BIT(2) || value & BIT(1))) {
+		writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+
+		usleep_range(10, 20);
+
+		writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+
+		value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
+		if (++count >= 1000) {
+			dev_err(core->dev, "LPI handshake timeout\n");
+			break;
+		}
+	}
+
 	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
 				 reg_val, reg_val & BIT(0), 200, 2000);
 	if (ret)
 		goto disable_power;
 
-	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
-	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+	writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
 
 	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
 	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);

---
base-commit: d968e50b5c26642754492dea23cbd3592bde62d8
change-id: 20250812-sm8650-power-sequence-fix-ba9a92098233

Best regards,
-- 
Dikshita Agarwal <quic_dikshita@quicinc.com>
Re: [PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Bryan O'Donoghue 1 month, 3 weeks ago 
On 12/08/2025 08:48, Dikshita Agarwal wrote:
> Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
> reading the LPI status register. Introduce a handshake loop to ensure
> MNoC enters low power mode reliably during VPU3 hardware power-off with
> timeout handling.

Can you confirm this is the sequence you want for sm8750 also ?

---
bod
Re: [PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Dikshita Agarwal 1 month, 3 weeks ago 

On 8/12/2025 2:30 PM, Bryan O'Donoghue wrote:
> On 12/08/2025 08:48, Dikshita Agarwal wrote:
>> Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
>> reading the LPI status register. Introduce a handshake loop to ensure
>> MNoC enters low power mode reliably during VPU3 hardware power-off with
>> timeout handling.
> 
> Can you confirm this is the sequence you want for sm8750 also ?

Yes, I have already mentioned in the SM8750 patches.

Thanks,
Dikshita
> 
> ---
> bod
Re: [PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Konrad Dybcio 1 month, 3 weeks ago 
On 8/12/25 9:48 AM, Dikshita Agarwal wrote:
> Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
> reading the LPI status register. Introduce a handshake loop to ensure
> MNoC enters low power mode reliably during VPU3 hardware power-off with
> timeout handling.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
>  drivers/media/platform/qcom/iris/iris_vpu3x.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> index 9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a..c2e6af575cbe4b3e3f2a019b24eecf3a5d469566 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> @@ -110,6 +110,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
>  static void iris_vpu33_power_off_hardware(struct iris_core *core)
>  {
>  	u32 reg_val = 0, value, i;
> +	u32 count = 0;
>  	int ret;
>  
>  	if (iris_vpu3x_hw_power_collapsed(core))
> @@ -128,13 +129,31 @@ static void iris_vpu33_power_off_hardware(struct iris_core *core)
>  			goto disable_power;
>  	}
>  
> +	/* set MNoC to low power */
> +	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +
> +	value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> +
> +	while (!(value & BIT(0)) && (value & BIT(2) || value & BIT(1))) {

It would be useful to introduce some defines for these bits, so that
we can reason about why bit 0 must always be set or neither bit 1 nor
bit 2 can be, in order to consider this done

> +		writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);

This write may be left unobserved by the hardware for quite some time,
but IIUC it's vital for the next write to make sense (i.e. you won't
meet the delay below as they may be buffered and reach the endpoint
essentially together)

> +
> +		usleep_range(10, 20);

https://www.kernel.org/doc/Documentation/timers/timers-howto.txt

suggests this should be an udelay, so that we can accurately reach
this (small) period

> +
> +		writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);

If these writes aren't actually necessary to be repeated a 1000 times,
you may want to use readl_poll_timeout

Konrad

> +
> +		value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> +		if (++count >= 1000) {
> +			dev_err(core->dev, "LPI handshake timeout\n");
> +			break;
> +		}
> +	}
> +
>  	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>  				 reg_val, reg_val & BIT(0), 200, 2000);
>  	if (ret)
>  		goto disable_power;
>  
> -	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
> -	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +	writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
>  
>  	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
>  	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> 
> ---
> base-commit: d968e50b5c26642754492dea23cbd3592bde62d8
> change-id: 20250812-sm8650-power-sequence-fix-ba9a92098233
> 
> Best regards,
Re: [PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Neil Armstrong 1 month, 3 weeks ago 
On 12/08/2025 09:48, Dikshita Agarwal wrote:
> Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
> reading the LPI status register. Introduce a handshake loop to ensure
> MNoC enters low power mode reliably during VPU3 hardware power-off with
> timeout handling.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
>   drivers/media/platform/qcom/iris/iris_vpu3x.c | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> index 9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a..c2e6af575cbe4b3e3f2a019b24eecf3a5d469566 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> @@ -110,6 +110,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
>   static void iris_vpu33_power_off_hardware(struct iris_core *core)
>   {
>   	u32 reg_val = 0, value, i;
> +	u32 count = 0;
>   	int ret;
>   
>   	if (iris_vpu3x_hw_power_collapsed(core))
> @@ -128,13 +129,31 @@ static void iris_vpu33_power_off_hardware(struct iris_core *core)
>   			goto disable_power;
>   	}
>   
> +	/* set MNoC to low power */
> +	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +
> +	value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> +
> +	while (!(value & BIT(0)) && (value & BIT(2) || value & BIT(1))) {
> +		writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +
> +		usleep_range(10, 20);
> +
> +		writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +
> +		value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> +		if (++count >= 1000) {
> +			dev_err(core->dev, "LPI handshake timeout\n");
> +			break;
> +		}
> +	}
> +
>   	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>   				 reg_val, reg_val & BIT(0), 200, 2000);
>   	if (ret)
>   		goto disable_power;
>   
> -	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
> -	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +	writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
>   
>   	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
>   	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> 
> ---
> base-commit: d968e50b5c26642754492dea23cbd3592bde62d8
> change-id: 20250812-sm8650-power-sequence-fix-ba9a92098233
> 
> Best regards,

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK

Thanks,
Neil
Re: [PATCH] media: iris: vpu3x: Add MNoC low power handshake during hardware power-off
Posted by Jorge Ramirez 1 month, 3 weeks ago 
On 12/08/25 10:06:40, Neil Armstrong wrote:
> On 12/08/2025 09:48, Dikshita Agarwal wrote:
> > Add the missing write to AON_WRAPPER_MVP_NOC_LPI_CONTROL before
> > reading the LPI status register. Introduce a handshake loop to ensure
> > MNoC enters low power mode reliably during VPU3 hardware power-off with
> > timeout handling.
> > 
> > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> > ---
> >   drivers/media/platform/qcom/iris/iris_vpu3x.c | 23 +++++++++++++++++++++--
> >   1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> > index 9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a..c2e6af575cbe4b3e3f2a019b24eecf3a5d469566 100644
> > --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> > +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> > @@ -110,6 +110,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
> >   static void iris_vpu33_power_off_hardware(struct iris_core *core)
> >   {
> >   	u32 reg_val = 0, value, i;
> > +	u32 count = 0;
> >   	int ret;
> >   	if (iris_vpu3x_hw_power_collapsed(core))
> > @@ -128,13 +129,31 @@ static void iris_vpu33_power_off_hardware(struct iris_core *core)
> >   			goto disable_power;
> >   	}
> > +	/* set MNoC to low power */
> > +	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> > +
> > +	value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> > +

for readibility purposes, could this code be restructed a bit
differently? maybe all the write/read operations in a single do while
loop with the exit conditions assigned to variables with an
identifiable meaning?

maybe also check/report errors outside the loop itself?


> > +	while (!(value & BIT(0)) && (value & BIT(2) || value & BIT(1))) {
> > +		writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> > +
> > +		usleep_range(10, 20);
> > +
> > +		writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> > +
> > +		value = readl(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS);
> > +		if (++count >= 1000) {
> > +			dev_err(core->dev, "LPI handshake timeout\n");
> > +			break;
> > +		}
> > +	}
> > +
> >   	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
> >   				 reg_val, reg_val & BIT(0), 200, 2000);
> >   	if (ret)
> >   		goto disable_power;
> > -	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
> > -	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> > +	writel(0, core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> >   	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
> >   	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> > 
> > ---
> > base-commit: d968e50b5c26642754492dea23cbd3592bde62d8
> > change-id: 20250812-sm8650-power-sequence-fix-ba9a92098233
> > 
> > Best regards,
> 
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK
> 
> Thanks,
> Neil
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.