Received: by 2002:a25:c593:0:0:0:0:0 with SMTP id v141csp5735247ybe; Tue, 10 Sep 2019 08:05:18 -0700 (PDT) X-Google-Smtp-Source: APXvYqyhy8iCuyPRskoYM1GHOGDjnKr/YVYSC2UlJNd5p7ayTWUCN9jOuqeWJZj3l6AzCCBUB8L6 X-Received: by 2002:a50:884c:: with SMTP id c12mr30998402edc.134.1568127918617; Tue, 10 Sep 2019 08:05:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1568127918; cv=none; d=google.com; s=arc-20160816; b=YCem9TgM7xUFxeKCDboXPY8UMryMyFM4UwXhra7QSAHUfMTqq8tkFbe3JGS2h+TSJc Wgkq22ZDpo7PsD2OL52cSmwYELmjwsof7gzK3MH+vnsjvT9Ga4nts7r3Yjp3OrhvqcPB GIQcGGY5ezRBzme87dX2mXmvOCHwU2p51IyK8Egk0efwlIuv51xco4Oi/r4t06R6/rhv iELazs4GwqSsziUVLNw5gAp+u/hgyfrsLTG8jWfrus5jFiHnBke7r7lRM0mNTgIAlrVo FcA2P2gbwOvoLuSjtPWRRIcKzQEEfbDtwtmVvUomG5bl+wzCcII7aSIDuV/EPTpoTReG nXVg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=30LVy5hLuIm0811eLjkgqaVyQxm9v1/8gPtTXtaVQbM=; b=JuL7xa9FuQn26K3ikpVN1fmdexw1IIK51LXUDsAwIlAxayyqwAs/JjSdWezPTb2zx1 MQZMpitn64M3zmCP/lpk71zkpLEFb3jJF1QsrhlvAxMHL7Tgzr6g9DDzJ+6j+r4LVzQT eiIU5dFOvF3s9qXmm0XEvzQQ8hGYsuT+3W/O4GNUm2a6H1JI7tHC9fo5HbGnWJiTgWqe 0XcHFkDAOsNHZ03gksulFdTMvBk16To1k5FeLN+C0kBaUVGumDSHn2v2788R0aH1Xoc3 xVttPgmTcWpqzj/EUEFvQwKjDhlOT+Ac+rA0ARGAuA5qHUVf8+YMKc7LYghqA9ZHF0eF Ljmg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 7si2058867ejz.151.2019.09.10.08.04.54; Tue, 10 Sep 2019 08:05:18 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2393446AbfIJPBl (ORCPT + 99 others); Tue, 10 Sep 2019 11:01:41 -0400 Received: from foss.arm.com ([217.140.110.172]:36866 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389308AbfIJPBl (ORCPT ); Tue, 10 Sep 2019 11:01:41 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 65DD228; Tue, 10 Sep 2019 08:01:40 -0700 (PDT) Received: from [10.1.197.57] (e110467-lin.cambridge.arm.com [10.1.197.57]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C724B3F71F; Tue, 10 Sep 2019 08:01:38 -0700 (PDT) Subject: Re: [PATCH] iommu/arm-smmu: fix "hang" when games exit To: Rob Clark , iommu@lists.linux-foundation.org Cc: linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Rob Clark , Will Deacon , Joerg Roedel , "moderated list:ARM SMMU DRIVERS" , open list References: <20190907175013.24246-1-robdclark@gmail.com> From: Robin Murphy Message-ID: <418d8426-f299-1269-2b2e-f86677cf22c2@arm.com> Date: Tue, 10 Sep 2019 16:01:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190907175013.24246-1-robdclark@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/09/2019 18:50, Rob Clark wrote: > From: Rob Clark > > When games, browser, or anything using a lot of GPU buffers exits, there > can be many hundreds or thousands of buffers to unmap and free. If the > GPU is otherwise suspended, this can cause arm-smmu to resume/suspend > for each buffer, resulting 5-10 seconds worth of reprogramming the > context bank (arm_smmu_write_context_bank()/arm_smmu_write_s2cr()/etc). > To the user it would appear that the system is locked up. > > A simple solution is to use pm_runtime_put_autosuspend() instead, so we > don't immediately suspend the SMMU device. > > Signed-off-by: Rob Clark > --- > Note: I've tied the autosuspend enable/delay to the consumer device, > based on the reasoning that if the consumer device benefits from using > an autosuspend delay, then it's corresponding SMMU probably does too. > Maybe that is overkill and we should just unconditionally enable > autosuspend. I'm not sure there's really any reason to expect that a supplier's usage model when doing things for itself bears any relation to that of its consumer(s), so I'd certainly lean towards the "unconditional" argument myself. Of course ideally we'd skip resuming altogether in the map/unmap paths (since resume implies a full TLB reset anyway), but IIRC that approach started to get messy in the context of the initial RPM patchset. I'm planning to fiddle around a bit more to clean up the implementation of the new iommu_flush_ops stuff, so I've made a note to myself to revisit RPM to see if there's a sufficiently clean way to do better. In the meantime, though, I don't have any real objection to using some reasonable autosuspend delay on the principle that if we've been woken up to map/unmap one page, there's a high likelihood that more will follow in short order (and in the configuration slow-paths it won't have much impact either way). Robin. > drivers/iommu/arm-smmu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index c2733b447d9c..73a0dd53c8a3 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -289,7 +289,7 @@ static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu) > static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu) > { > if (pm_runtime_enabled(smmu->dev)) > - pm_runtime_put(smmu->dev); > + pm_runtime_put_autosuspend(smmu->dev); > } > > static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) > @@ -1445,6 +1445,15 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) > /* Looks ok, so add the device to the domain */ > ret = arm_smmu_domain_add_master(smmu_domain, fwspec); > > +#ifdef CONFIG_PM > + /* TODO maybe device_link_add() should do this for us? */ > + if (dev->power.use_autosuspend) { > + pm_runtime_set_autosuspend_delay(smmu->dev, > + dev->power.autosuspend_delay); > + pm_runtime_use_autosuspend(smmu->dev); > + } > +#endif > + > rpm_put: > arm_smmu_rpm_put(smmu); > return ret; >