Re: [PATCH v8 08/12] iommu/tegra241-cmdqv: Add a helper to quiesce VCMDQs

[Pranjal Shrivastava <praan@google.com>]

On Mon, Jun 01, 2026 at 10:59:33PM -0700, Nicolin Chen wrote:
> On Tue, Jun 02, 2026 at 03:37:43AM +0000, Pranjal Shrivastava wrote:
> > On Mon, Jun 01, 2026 at 05:14:24PM -0700, Nicolin Chen wrote:
> > > On Mon, Jun 01, 2026 at 09:59:05PM +0000, Pranjal Shrivastava wrote:
> > > > @@ -887,7 +887,7 @@ struct arm_smmu_impl_ops {
> > > >  	size_t (*get_viommu_size)(enum iommu_viommu_type viommu_type);
> > > >  	int (*vsmmu_init)(struct arm_vsmmu *vsmmu,
> > > >  			  const struct iommu_user_data *user_data);
> > > > -	int (*drain_queues)(struct arm_smmu_device *smmu);
> > > > +	int (*quiesce_and_drain_queues)(struct arm_smmu_device *smmu);
> > > 
> > > Hmm? What's the point in PATCH-3 adding drain_queues then?
> > 
> > Well, you suggested in the previous version that STOP_FLAG needs to be
> > set for the secondary CMDQs too
> > 
> > All we're doing here is renaming this function as it also sets the
> > STOP_FLAG on impl-specific CMDQs now. We can still call it drain_queues
> > if you prefer not renaming this.
> > 
> > On the other hand, we can add a dedicated queisce_queues() op too that's
> > called before draining queues. I found that to be unnecessary as I'd rather
> > have one impl-specific op handle everything for impl-specific queues.
> 
> The renaming is very confusing... I'd leave it as drain_queues.

Ack.

> 
> Arguably,
> 	atomic_or(CMDQ_PROD_STOP_FLAG, &vcmdq->cmdq.q.llq.atomic.prod);
> could be even stuffed into tegra241_cmdqv_drain_vintf0_lvcmdqs(),
> where there is a for loop already.

That would be slow IMO, because we'd set the the STOP_FLAG on ONE queue,
then wait for it to drain and repeat this pattern for all vCMDQs.
Ideally, we should set the STOP_FLAG on all vCMDQs first and then wait
for them to drain.

Thanks,
Praan