On Mon, Aug 7, 2017 at 4:27 AM, Vivek Gautam
[off-list ref] wrote:

On Thu, Jul 13, 2017 at 5:20 PM, Rob Clark [off-list ref] wrote:

quoted

On Thu, Jul 13, 2017 at 1:35 AM, Sricharan R [off-list ref] wrote:

quoted

Hi Vivek,

On 7/13/2017 10:43 AM, Vivek Gautam wrote:

quoted

Hi Stephen,


On 07/13/2017 04:24 AM, Stephen Boyd wrote:

quoted

On 07/06, Vivek Gautam wrote:

quoted

@@ -1231,12 +1237,18 @@ static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
  static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
                   size_t size)
  {
-    struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+    struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+    struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
+    size_t ret;
        if (!ops)
          return 0;
  -    return ops->unmap(ops, iova, size);
+    pm_runtime_get_sync(smmu_domain->smmu->dev);

Can these map/unmap ops be called from an atomic context? I seem
to recall that being a problem before.

That's something which was dropped in the following patch merged in master:
523d7423e21b iommu/arm-smmu: Remove io-pgtable spinlock

Looks like we don't  need locks here anymore?

 Apart from the locking, wonder why a explicit pm_runtime is needed
 from unmap. Somehow looks like some path in the master using that
 should have enabled the pm ?

Yes, there are a bunch of scenarios where unmap can happen with
disabled master (but not in atomic context).

I would like to understand whether there is a situation where an unmap is
called in atomic context without an enabled master?

Let's say we have the case where all the unmap calls in atomic context happen
only from the master's context (in which case the device link should
take care of
the pm state of smmu), and the only unmap that happen in non-atomic context
is the one with master disabled. In such a case doesn it make sense to
distinguish
the atomic/non-atomic context and add pm_runtime_get_sync()/put_sync() only
for the non-atomic context since that would be the one with master disabled.

At least drm/msm needs to hold obj->lock (a mutex) in unmap, so it
won't unmap anything in atomic ctx (but it can unmap w/ master
disabled).  I can't really comment about other non-gpu drivers.  It
seems like a reasonable constraint that either master is enabled or
not in atomic ctx.

Currently we actually wrap unmap w/ pm_runtime_get/put_sync(), but I'd
like to drop that to avoid powering up the gpu.

BR,
-R