Re: [RFC PATCH 7/8] iommu/arm-smmu-v3: Invoke rpm operation before accessing the hw

[Pranjal Shrivastava <praan@google.com>]

On Mon, Jun 16, 2025 at 10:56:13AM +0800, Xueqi Zhang wrote:

Hi Xueqi,

> Invoke rpm operation before accessing the SMMU hw.
> 
> Signed-off-by: Xueqi Zhang <xueqi.zhang@mediatek.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 84 ++++++++++++++++++++-
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  3 +
>  2 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 154417b380fa..88912b0f8132 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -122,6 +122,22 @@ static void parse_driver_options(struct arm_smmu_device *smmu)
>  	} while (arm_smmu_options[++i].opt);
>  }
>  
> +static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> +{
> +	if (smmu && smmu->impl && smmu->impl->smmu_power_get)
> +		return smmu->impl->smmu_power_get(smmu);
> +
> +	return 0;
> +}
> +
> +static int arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> +{
> +	if (smmu && smmu->impl && smmu->impl->smmu_power_put)
> +		return smmu->impl->smmu_power_put(smmu);
> +
> +	return 0;
> +}
> +

I've been working on enabling PM runtime for arm-smmu-v3 for a while, 
I just posted the RFCv3 for that series [1]. I see that you need some
implementation specific rpm calls too, I think it would be nice if we
could align on this?

Perhaps, you could rebase this on top of my series OR we can collaborate
for the runtime PM series where you can contribute only the rpm patches
from this series to handle your implmentation? 
Let me know what you think! 

>  /* Low-level queue manipulation functions */
>  static bool queue_has_space(struct arm_smmu_ll_queue *q, u32 n)
>  {
> @@ -2082,23 +2098,35 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev)
>  static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
>  {
>  	struct arm_smmu_device *smmu = dev;
> +	int ret;
> +
> +	ret = arm_smmu_rpm_get(smmu);
> +	if (ret)
> +		return IRQ_NONE;
>  
>  	arm_smmu_evtq_thread(irq, dev);
>  	if (smmu->features & ARM_SMMU_FEAT_PRI)
>  		arm_smmu_priq_thread(irq, dev);
>  
> +	arm_smmu_rpm_put(smmu);
>  	return IRQ_HANDLED;
>  }
>  
>  static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
>  {
> +
> +	ret = arm_smmu_rpm_get(smmu);
> +	if (ret)
> +		return IRQ_WAKE_THREAD;
>  
>  	arm_smmu_gerror_handler(irq, dev);
>  
>  	if (smmu->impl && smmu->impl->combined_irq_handle)
>  		smmu->impl->combined_irq_handle(irq, smmu);
>  
> +	arm_smmu_rpm_put(smmu);
>  	return IRQ_WAKE_THREAD;
>  }
>  
> @@ -2255,6 +2283,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>  	struct arm_smmu_domain *smmu_domain = cookie;
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>  	struct arm_smmu_cmdq_ent cmd;
> +	int ret;
> +
> +	ret = arm_smmu_rpm_get(smmu);
> +	if (ret)
> +		return;
>  
>  	/*
>  	 * NOTE: when io-pgtable is in non-strict mode, we may get here with
> @@ -2271,6 +2304,8 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>  		arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
>  	}
>  	arm_smmu_atc_inv_domain(smmu_domain, 0, 0);
> +
> +	arm_smmu_rpm_put(smmu);
>  }
>  
>  static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
> @@ -2353,6 +2388,11 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
>  			.leaf	= leaf,
>  		},
>  	};
> +	int ret;
> +
> +	ret = arm_smmu_rpm_get(smmu_domain->smmu);
> +	if (ret)
> +		return;

I'm afraid we aren't going for a hard rpm_get in such functions in our
design as per the the discussions in the rpm series[1]. It would be 
great if you could review that too and drop some comments there, I'd be
happy to understand and collaborate to meet your requirements as well :)

[...]

>  static const struct of_device_id arm_smmu_of_match[] = {
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index f45c4bf84bc1..cd96ff9cbc54 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -792,6 +792,7 @@ struct arm_smmu_device {
>  
>  	struct rb_root			streams;
>  	struct mutex			streams_mutex;
> +
>  	const struct arm_smmu_v3_impl	*impl;
>  };
>  
> @@ -1004,6 +1005,8 @@ struct arm_smmu_v3_impl {
>  	int (*combined_irq_handle)(int irq, struct arm_smmu_device *smmu_dev);
>  	int (*smmu_evt_handler)(int irq, struct arm_smmu_device *smmu_dev,
>  				u64 *evt, struct ratelimit_state *rs);
> +	int (*smmu_power_get)(struct arm_smmu_device *smmu);
> +	int (*smmu_power_put)(struct arm_smmu_device *smmu);
>  };
>  
>  struct arm_smmu_device *arm_smmu_v3_impl_init(struct arm_smmu_device *smmu);
> -- 
> 2.46.0
> 

Thanks,
Praan

[1] https://lore.kernel.org/all/20250616203149.2649118-1-praan@google.com/