From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [Freedreno] [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers Date: Thu, 15 Feb 2018 13:09:03 +0900 Message-ID: References: <1517999482-17317-1-git-send-email-vivek.gautam@codeaurora.org> <1517999482-17317-7-git-send-email-vivek.gautam@codeaurora.org> <20180213164205.GA7599@jcrouse-lnx.qualcomm.com> <20180214154850.GA25422@jcrouse-lnx.qualcomm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: Mark Rutland , devicetree@vger.kernel.org, Linux PM , David Airlie , "Rafael J. Wysocki" , linux-arm-msm , Will Deacon , Linux Kernel Mailing List , dri-devel , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Rob Herring , Vivek Gautam , Greg KH , freedreno , Stephen Boyd , Robin Murphy List-Id: linux-arm-msm@vger.kernel.org T24gVGh1LCBGZWIgMTUsIDIwMTggYXQgMToxMiBBTSwgUm9iIENsYXJrIDxyb2JkY2xhcmtAZ21h aWwuY29tPiB3cm90ZToKPiBPbiBXZWQsIEZlYiAxNCwgMjAxOCBhdCAxMDo0OCBBTSwgSm9yZGFu IENyb3VzZSA8amNyb3VzZUBjb2RlYXVyb3JhLm9yZz4gd3JvdGU6Cj4+IE9uIFdlZCwgRmViIDE0 LCAyMDE4IGF0IDEyOjMxOjI5UE0gKzA5MDAsIFRvbWFzeiBGaWdhIHdyb3RlOgo+Pj4KPj4+IC0g V2hlbiBzdWJtaXR0aW5nIGNvbW1hbmRzIHRvIHRoZSBHUFUsIHRoZSBHUFUgZHJpdmVyIHdpbGwK Pj4+IHBtX3J1bnRpbWVfZ2V0X3N5bmMoKSBvbiB0aGUgR1BVIGRldmljZSwgd2hpY2ggd2lsbCBh dXRvbWF0aWNhbGx5IGRvCj4+PiB0aGUgc2FtZSBvbiBhbGwgdGhlIGxpbmtlZCBzdXBwbGllcnMs IHdoaWNoIHdvdWxkIGFsc28gaW5jbHVkZSB0aGUKPj4+IFNNTVUgaXRzZWxmLiBUaGUgcm9sZSBv ZiBkZXZpY2UgbGlua3MgaGVyZSBpcyBleGFjdGx5IHRoYXQgdGhlIEdQVQo+Pj4gZHJpdmVyIGRv ZXNuJ3QgaGF2ZSB0byBjYXJlIHdoaWNoIG90aGVyIGRldmljZXMgbmVlZCB0byBiZSBicm91Z2h0 IHVwLgo+Pgo+PiBUaGlzIGlzIHRydWUuICBBc3N1bWluZyB0aGF0IHRoZSBkZXZpY2UgbGluayB3 b3JrcyBjb3JyZWN0bHkgd2Ugd291bGQgbm90IG5lZWQKPj4gdG8gZXhwbGljaXRseSBwb3dlciB0 aGUgU01NVSB3aGljaCBtYWtlcyBteSBwb2ludCBlbnRpcmVseSBtb290Lgo+Cj4gSnVzdCB0byBw b2ludCBvdXQgd2hhdCBtb3RpdmF0ZWQgdGhpcyBwYXRjaHNldCwgdGhlIGJpZ2dlc3QgcHJvYmxl bSBpcwo+IGlvbW11X3VubWFwKCkgYmVjYXVzZSB0aGF0IGNhbiBoYXBwZW4gd2hlbiBHUFUgaXMg bm90IHBvd2VyZWQgb24gKG9yCj4gaW4gdGhlIHY0bDIgY2FzZSwgYmVjYXVzZSBzb21lIG90aGVy IGRldmljZSBkcm9wcGVkIGl0J3MgcmVmZXJlbmNlIHRvCj4gdGhlIGRtYS1idWYgYWxsb3dpbmcg aXQgdG8gYmUgZnJlZSdkKS4gIEN1cnJlbnRseSB3ZSBwbSBnZXQvcHV0IHRoZQo+IEdQVSBkZXZp Y2UgYXJvdW5kIHVubWFwLCBidXQgaXQgaXMga2luZGEgc2lsbHkgdG8gYm9vdCB1cCB0aGUgR1BV IGp1c3QKPiB0byB1bm1hcCBhIGJ1ZmZlci4KCk5vdGUgdGhhdCBpbiBWNEwyIGJvdGggbWFwcGlu ZyBhbmQgdW5tYXBwaW5nIGNhbiBoYXBwZW4gY29tcGxldGVseQp3aXRob3V0IGludm9sdmluZyB0 aGUgZHJpdmVyLiBTbyBBRkFJQ1QgdGhlIGFwcHJvYWNoIGJlaW5nIGltcGxlbWVudGVkCmJ5IHRo aXMgcGF0Y2hzZXQgd2lsbCBub3Qgd29yaywgYmVjYXVzZSB0aGVyZSB3aWxsIGJlIG5vIG9uZSB0 byBwb3dlcgp1cCB0aGUgSU9NTVUgYmVmb3JlIHRoZSBvcGVyYXRpb24uIE1vcmVvdmVyLCB0aGVy ZSBhcmUgcGxhdGZvcm1zIGZvcgp3aGljaCB0aGVyZSBpcyBubyByZWFzb24gdG8gcG93ZXIgdXAg dGhlIElPTU1VIGp1c3QgZm9yIG1hcC91bm1hcCwKYmVjYXVzZSB0aGUgaGFyZHdhcmUgc3RhdGUg aXMgbG9zdCBhbnl3YXkgYW5kIHRoZSBvbmx5IHJlYWwgd29yawpuZWVkZWQgaXMgdXBkYXRpbmcg dGhlIHBhZ2UgdGFibGVzIGluIG1lbW9yeS4gKEkgZmVlbCBsaWtlIHRoaXMgaXMKYWN0dWFsbHkg dHJ1ZSBmb3IgbW9zdCBvZiB0aGUgcGxhdGZvcm1zIGluIHRoZSB3aWxkLCBidXQgdGhpcyBpcyBi YXNlZApwdXJlbHkgb24gdGhlIG5vdCBzbyBzbWFsbCBudW1iZXIgb2YgcGxhdGZvcm1zIEkgd29y a2VkIHdpdGgsIGhhdmVuJ3QKYm90aGVyZWQgbG9va2luZyBmb3IgbW9yZSBnZW5lcmFsIGV2aWRl bmNlLikKCj4KPiAoU2VtaS1yZWxhdGVkLCBJIHdvdWxkIGFsc28gbGlrZSB0byBiYXRjaCBtYXAv dW5tYXAncywgSSBqdXN0IGhhdmVuJ3QKPiBnb3R0ZW4gYXJvdW5kIHRvIGltcGxlbWVudGluZyBp dCB5ZXQuLiBidXQgdGhhdCB3b3VsZCBiZSBhbm90aGVyIGNhc2UKPiB3aGVyZSBhIHNpbmdsZSBn ZXRfc3VwcGxpZXIoKS9wdXRfc3VwcGxpZXIoKSBvdXRzaWRlIG9mIHRoZSBpb21tdQo+IHdvdWxk IG1ha2Ugc2Vuc2UgaW5zdGVhZCBvZiBwbV9nZXQvcHV0KCkgaW5zaWRlIHRoZSBpb21tdSBkcml2 ZXIncwo+IC0+dW5tYXAoKS4pCj4KPiBJZiB5b3UgcmVhbGx5IGRpc2xpa2UgdGhlIGdldC9wdXRf c3VwcGxpZXIoKSBhcHByb2FjaCwgdGhlbiBwZXJoYXBzIHdlCj4gbmVlZCBpb21tdV9wbV9nZXQo KS9pb21tdV9wbV9wdXQoKSBvcGVyYXRpb25zIHRoYXQgdGhlIGlvbW11IHVzZXIKPiBjb3VsZCB1 c2UgdG8gYWNjb21wbGlzaCB0aGUgc2FtZSB0aGluZz8KCkknbSBhZnJhaWQgdGhpcyB3b3VsZG4n dCB3b3JrIGZvciBWNEwyIGVpdGhlci4gQW5kIEkgc3RpbGwgaGF2ZW4ndApiZWVuIGdpdmVuIGFu eSBldmlkZW5jZSB0aGF0IHRoZSBhcHByb2FjaCBJJ20gc3VnZ2VzdGluZywgd2hpY2ggcmVsaWVz Cm9ubHkgb24gZXhpc3RpbmcgcGllY2VzIG9mIGluZnJhc3RydWN0dXJlIGFuZCB3aGljaCB3b3Jr ZWQgZm9yIGJvdGgKRXh5bm9zIGFuZCBSb2NrY2hpcCwgaW5jbHVkaW5nIFY0TDIsIHdvdWxkbid0 IHdvcmsgZm9yIFNNTVUgYW5kL29yIFFDClNvQ3MuCgpCZXN0IHJlZ2FyZHMsClRvbWFzegpfX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFp bGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1518667766; cv=none; d=google.com; s=arc-20160816; b=RPeQFUEoi+9mIeSPmq+klj9d88OmG5fuUAL60g+JdLJuFKaVJM1JHpWpPnbuFokECq eU2I4/Oo41jjzoVxI5zd5zwwNz9bInsSUdcHQXzx3Vn+GYoxed3WN6F3f+Cl3ncfojDT nXnci1dF/+IaQleNlmHcicTPVvFfFXx/Ip4LL2Fp6zADkyBpSY6jvApHEfo1ugEnv6aw 4lIQqlfgABZvbi+NO+MxcyCYagBfjBt6zDmqcTthNYfWV4szfSME+5/lzUz7JS/cl61L gew7hqPwsVy/oYINH048FyiIISAirm/bNp4z+gtEtgEVxUSL9dCzz8R0KENvDrcouT8g nkYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:dkim-signature:arc-authentication-results; bh=f4VkzchlQLVppKlHMNFOtIBpaRzxm370ANoaRhKFGa4=; b=eVfHuw4UqZYGAAR5lgQGqk4yt9V3EAj96/1vpgZ4NNBH58WJqUvZ/J1GGIr4dC/SmL NXaQH9rCihbRHTyxs4SDgrmMDWa/Vr9UIqa+x9/wq7fTL2SRuvX4jvKPC/cSHt4HSSc6 jA7j4pdaYIfXaBYzHXYedbN8bamFvWWRxS3vVwE9DL81K3iqxtj8QFEB6/66sJWc7vyn zUx/eEwnZXIO+oSKa746aeIvXvlU7Gr3I2lWbyoEk263rka+9HeLDRl/jfGy5Jo3g2Kn oR7sBkdzlOz8o42ziPg0a0JQbJ0hkZ6N9CVUkzhMRbFBBpnt5VhTMaXda1LWwvP8qSEq EJBA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=NYGNsI2G; spf=pass (google.com: domain of tfiga@chromium.org designates 209.85.220.41 as permitted sender) smtp.mailfrom=tfiga@chromium.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=NYGNsI2G; spf=pass (google.com: domain of tfiga@chromium.org designates 209.85.220.41 as permitted sender) smtp.mailfrom=tfiga@chromium.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org X-Google-Smtp-Source: AH8x227zcAIb1c3emh8u8x0YJ6R8ca8wjVgdVFR5lPfZzokjSkdkA2nyjTnhhM33dOIiKot3Duew1w== MIME-Version: 1.0 In-Reply-To: References: <1517999482-17317-1-git-send-email-vivek.gautam@codeaurora.org> <1517999482-17317-7-git-send-email-vivek.gautam@codeaurora.org> <20180213164205.GA7599@jcrouse-lnx.qualcomm.com> <20180214154850.GA25422@jcrouse-lnx.qualcomm.com> From: Tomasz Figa Date: Thu, 15 Feb 2018 13:09:03 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [Freedreno] [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers To: Rob Clark Cc: Vivek Gautam , Mark Rutland , devicetree@vger.kernel.org, Linux PM , David Airlie , Will Deacon , "Rafael J. Wysocki" , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , dri-devel , Linux Kernel Mailing List , Rob Herring , Greg KH , freedreno , Robin Murphy , Stephen Boyd , linux-arm-msm Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1591737886832187485?= X-GMAIL-MSGID: =?utf-8?q?1592438571703048133?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Thu, Feb 15, 2018 at 1:12 AM, Rob Clark wrote: > On Wed, Feb 14, 2018 at 10:48 AM, Jordan Crouse wrote: >> On Wed, Feb 14, 2018 at 12:31:29PM +0900, Tomasz Figa wrote: >>> >>> - When submitting commands to the GPU, the GPU driver will >>> pm_runtime_get_sync() on the GPU device, which will automatically do >>> the same on all the linked suppliers, which would also include the >>> SMMU itself. The role of device links here is exactly that the GPU >>> driver doesn't have to care which other devices need to be brought up. >> >> This is true. Assuming that the device link works correctly we would not need >> to explicitly power the SMMU which makes my point entirely moot. > > Just to point out what motivated this patchset, the biggest problem is > iommu_unmap() because that can happen when GPU is not powered on (or > in the v4l2 case, because some other device dropped it's reference to > the dma-buf allowing it to be free'd). Currently we pm get/put the > GPU device around unmap, but it is kinda silly to boot up the GPU just > to unmap a buffer. Note that in V4L2 both mapping and unmapping can happen completely without involving the driver. So AFAICT the approach being implemented by this patchset will not work, because there will be no one to power up the IOMMU before the operation. Moreover, there are platforms for which there is no reason to power up the IOMMU just for map/unmap, because the hardware state is lost anyway and the only real work needed is updating the page tables in memory. (I feel like this is actually true for most of the platforms in the wild, but this is based purely on the not so small number of platforms I worked with, haven't bothered looking for more general evidence.) > > (Semi-related, I would also like to batch map/unmap's, I just haven't > gotten around to implementing it yet.. but that would be another case > where a single get_supplier()/put_supplier() outside of the iommu > would make sense instead of pm_get/put() inside the iommu driver's > ->unmap().) > > If you really dislike the get/put_supplier() approach, then perhaps we > need iommu_pm_get()/iommu_pm_put() operations that the iommu user > could use to accomplish the same thing? I'm afraid this wouldn't work for V4L2 either. And I still haven't been given any evidence that the approach I'm suggesting, which relies only on existing pieces of infrastructure and which worked for both Exynos and Rockchip, including V4L2, wouldn't work for SMMU and/or QC SoCs. Best regards, Tomasz