From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Clark Subject: Re: [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers Date: Thu, 15 Feb 2018 09:14:40 -0500 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: freedreno-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Freedreno" To: Tomasz Figa Cc: Mark Rutland , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux PM , David Airlie , "Rafael J. Wysocki" , linux-arm-msm , Will Deacon , Linux Kernel Mailing List , dri-devel , "list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS , Joerg Roedel , " , Rob Herring , Vivek Gautam , Greg KH , freedreno , Stephen Boyd , Robin Murphy List-Id: linux-arm-msm@vger.kernel.org T24gV2VkLCBGZWIgMTQsIDIwMTggYXQgMTE6MDkgUE0sIFRvbWFzeiBGaWdhIDx0ZmlnYUBjaHJv bWl1bS5vcmc+IHdyb3RlOgo+IE9uIFRodSwgRmViIDE1LCAyMDE4IGF0IDE6MTIgQU0sIFJvYiBD bGFyayA8cm9iZGNsYXJrQGdtYWlsLmNvbT4gd3JvdGU6Cj4+IE9uIFdlZCwgRmViIDE0LCAyMDE4 IGF0IDEwOjQ4IEFNLCBKb3JkYW4gQ3JvdXNlIDxqY3JvdXNlQGNvZGVhdXJvcmEub3JnPiB3cm90 ZToKPj4+IE9uIFdlZCwgRmViIDE0LCAyMDE4IGF0IDEyOjMxOjI5UE0gKzA5MDAsIFRvbWFzeiBG aWdhIHdyb3RlOgo+Pj4+Cj4+Pj4gLSBXaGVuIHN1Ym1pdHRpbmcgY29tbWFuZHMgdG8gdGhlIEdQ VSwgdGhlIEdQVSBkcml2ZXIgd2lsbAo+Pj4+IHBtX3J1bnRpbWVfZ2V0X3N5bmMoKSBvbiB0aGUg R1BVIGRldmljZSwgd2hpY2ggd2lsbCBhdXRvbWF0aWNhbGx5IGRvCj4+Pj4gdGhlIHNhbWUgb24g YWxsIHRoZSBsaW5rZWQgc3VwcGxpZXJzLCB3aGljaCB3b3VsZCBhbHNvIGluY2x1ZGUgdGhlCj4+ Pj4gU01NVSBpdHNlbGYuIFRoZSByb2xlIG9mIGRldmljZSBsaW5rcyBoZXJlIGlzIGV4YWN0bHkg dGhhdCB0aGUgR1BVCj4+Pj4gZHJpdmVyIGRvZXNuJ3QgaGF2ZSB0byBjYXJlIHdoaWNoIG90aGVy IGRldmljZXMgbmVlZCB0byBiZSBicm91Z2h0IHVwLgo+Pj4KPj4+IFRoaXMgaXMgdHJ1ZS4gIEFz c3VtaW5nIHRoYXQgdGhlIGRldmljZSBsaW5rIHdvcmtzIGNvcnJlY3RseSB3ZSB3b3VsZCBub3Qg bmVlZAo+Pj4gdG8gZXhwbGljaXRseSBwb3dlciB0aGUgU01NVSB3aGljaCBtYWtlcyBteSBwb2lu dCBlbnRpcmVseSBtb290Lgo+Pgo+PiBKdXN0IHRvIHBvaW50IG91dCB3aGF0IG1vdGl2YXRlZCB0 aGlzIHBhdGNoc2V0LCB0aGUgYmlnZ2VzdCBwcm9ibGVtIGlzCj4+IGlvbW11X3VubWFwKCkgYmVj YXVzZSB0aGF0IGNhbiBoYXBwZW4gd2hlbiBHUFUgaXMgbm90IHBvd2VyZWQgb24gKG9yCj4+IGlu IHRoZSB2NGwyIGNhc2UsIGJlY2F1c2Ugc29tZSBvdGhlciBkZXZpY2UgZHJvcHBlZCBpdCdzIHJl ZmVyZW5jZSB0bwo+PiB0aGUgZG1hLWJ1ZiBhbGxvd2luZyBpdCB0byBiZSBmcmVlJ2QpLiAgQ3Vy cmVudGx5IHdlIHBtIGdldC9wdXQgdGhlCj4+IEdQVSBkZXZpY2UgYXJvdW5kIHVubWFwLCBidXQg aXQgaXMga2luZGEgc2lsbHkgdG8gYm9vdCB1cCB0aGUgR1BVIGp1c3QKPj4gdG8gdW5tYXAgYSBi dWZmZXIuCj4KPiBOb3RlIHRoYXQgaW4gVjRMMiBib3RoIG1hcHBpbmcgYW5kIHVubWFwcGluZyBj YW4gaGFwcGVuIGNvbXBsZXRlbHkKPiB3aXRob3V0IGludm9sdmluZyB0aGUgZHJpdmVyLiBTbyBB RkFJQ1QgdGhlIGFwcHJvYWNoIGJlaW5nIGltcGxlbWVudGVkCj4gYnkgdGhpcyBwYXRjaHNldCB3 aWxsIG5vdCB3b3JrLCBiZWNhdXNlIHRoZXJlIHdpbGwgYmUgbm8gb25lIHRvIHBvd2VyCj4gdXAg dGhlIElPTU1VIGJlZm9yZSB0aGUgb3BlcmF0aW9uLiBNb3Jlb3ZlciwgdGhlcmUgYXJlIHBsYXRm b3JtcyBmb3IKPiB3aGljaCB0aGVyZSBpcyBubyByZWFzb24gdG8gcG93ZXIgdXAgdGhlIElPTU1V IGp1c3QgZm9yIG1hcC91bm1hcCwKPiBiZWNhdXNlIHRoZSBoYXJkd2FyZSBzdGF0ZSBpcyBsb3N0 IGFueXdheSBhbmQgdGhlIG9ubHkgcmVhbCB3b3JrCj4gbmVlZGVkIGlzIHVwZGF0aW5nIHRoZSBw YWdlIHRhYmxlcyBpbiBtZW1vcnkuIChJIGZlZWwgbGlrZSB0aGlzIGlzCj4gYWN0dWFsbHkgdHJ1 ZSBmb3IgbW9zdCBvZiB0aGUgcGxhdGZvcm1zIGluIHRoZSB3aWxkLCBidXQgdGhpcyBpcyBiYXNl ZAo+IHB1cmVseSBvbiB0aGUgbm90IHNvIHNtYWxsIG51bWJlciBvZiBwbGF0Zm9ybXMgSSB3b3Jr ZWQgd2l0aCwgaGF2ZW4ndAo+IGJvdGhlcmVkIGxvb2tpbmcgZm9yIG1vcmUgZ2VuZXJhbCBldmlk ZW5jZS4pCj4KCkF0IGxlYXN0IGFzIGZhciBhcyBkcm0vbXNtL2FkcmVubywgSSdtIG5vdCB0ZXJy aWJseSBjb25jZXJuZWQgYWJvdXQKb3RoZXIgcGxhdGZvcm1zIHRoYXQgZG9uJ3QgbmVlZCB0byBw b3dlciB1cCBpb21tdS4gIEl0J3Mgbm90IHJlYWxseQp0aGUgc2FtZSBzaXR1YXRpb24gYXMgYSBJ UCBibG9jayB0aGF0IHNob3dzIHVwIGluIGFsbCBkaWZmZXJlbnQKdmVuZG9yJ3MgU29Dcy4KCkJ1 dCBpZiB5b3UgY2FuIGNvbnZpbmNlIFJvYmluIHRvIGdvIGZvciBnZXQvcHV0X3N5bmMoKSBjYWxs cyBpbnNpZGUKdGhlIGlvbW11IGRyaXZlciwgSSdtIGZpbmUgd2l0aCB0aGF0IGFwcHJvYWNoIHRv by4gIFRoYXQgaXMgd2hhdCBJIGRvCmluIHFjb21faW9tbXUgYWxyZWFkeS4gIEJ1dCBpZiBub3Qg SSdkIGxpa2UgdG8gYXQgbGVhc3Qgc29sdmUgdGhpcyBmb3IKc29tZSBwbGF0Zm9ybXMgaWYgd2Ug Y2FuJ3Qgc29sdmUgZm9yIGFsbC4KCkJSLAotUgoKPj4KPj4gKFNlbWktcmVsYXRlZCwgSSB3b3Vs ZCBhbHNvIGxpa2UgdG8gYmF0Y2ggbWFwL3VubWFwJ3MsIEkganVzdCBoYXZlbid0Cj4+IGdvdHRl biBhcm91bmQgdG8gaW1wbGVtZW50aW5nIGl0IHlldC4uIGJ1dCB0aGF0IHdvdWxkIGJlIGFub3Ro ZXIgY2FzZQo+PiB3aGVyZSBhIHNpbmdsZSBnZXRfc3VwcGxpZXIoKS9wdXRfc3VwcGxpZXIoKSBv dXRzaWRlIG9mIHRoZSBpb21tdQo+PiB3b3VsZCBtYWtlIHNlbnNlIGluc3RlYWQgb2YgcG1fZ2V0 L3B1dCgpIGluc2lkZSB0aGUgaW9tbXUgZHJpdmVyJ3MKPj4gLT51bm1hcCgpLikKPj4KPj4gSWYg eW91IHJlYWxseSBkaXNsaWtlIHRoZSBnZXQvcHV0X3N1cHBsaWVyKCkgYXBwcm9hY2gsIHRoZW4g cGVyaGFwcyB3ZQo+PiBuZWVkIGlvbW11X3BtX2dldCgpL2lvbW11X3BtX3B1dCgpIG9wZXJhdGlv bnMgdGhhdCB0aGUgaW9tbXUgdXNlcgo+PiBjb3VsZCB1c2UgdG8gYWNjb21wbGlzaCB0aGUgc2Ft ZSB0aGluZz8KPgo+IEknbSBhZnJhaWQgdGhpcyB3b3VsZG4ndCB3b3JrIGZvciBWNEwyIGVpdGhl ci4gQW5kIEkgc3RpbGwgaGF2ZW4ndAo+IGJlZW4gZ2l2ZW4gYW55IGV2aWRlbmNlIHRoYXQgdGhl IGFwcHJvYWNoIEknbSBzdWdnZXN0aW5nLCB3aGljaCByZWxpZXMKPiBvbmx5IG9uIGV4aXN0aW5n IHBpZWNlcyBvZiBpbmZyYXN0cnVjdHVyZSBhbmQgd2hpY2ggd29ya2VkIGZvciBib3RoCj4gRXh5 bm9zIGFuZCBSb2NrY2hpcCwgaW5jbHVkaW5nIFY0TDIsIHdvdWxkbid0IHdvcmsgZm9yIFNNTVUg YW5kL29yIFFDCj4gU29Dcy4KPgo+IEJlc3QgcmVnYXJkcywKPiBUb21hc3oKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KRnJlZWRyZW5vIG1haWxpbmcgbGlz dApGcmVlZHJlbm9AbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZnJlZWRyZW5vCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1518704081; cv=none; d=google.com; s=arc-20160816; b=WDc9+QqvYyajg2/3Kve+BFOMO6/cqkHhxAae7BmT8zj4ljSrj5pJ3EVL+nZqp/w/8w QaXINfvu6lDKMAi9K0z8SdjWvKFqDuhcv30mQ8qPUITg0fNEwqNeZHYwhB9k1h1s+uAy RLohyBfV0AOZaBdt+aBPSvIgeKfEK09uw2Imo5qIzmJBwGQ9z3NJOj7z6IEpJd7T1KtZ weoiM9gVwWoVYO+Aq+xyiQXUb7TpztVt9QwTJdgC0t4LjQMZlbW2j+6+/FxVeBrl0JP9 2VUdAp6iox6L5uQBKal+KFkwMFscx0zRqskm+tOEBgrrlj6VUYOhAdeX0MlrlrIc4uQS t6gg== 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=FOmISjBC6Is5ZjZWghzdW9qvlburSkxvMszTs4qCx3M=; b=BX/NTJTO7JXL9+I5KBmWVxaGseh3qK9SFAPpdCnNAoCx/53PITASWpnKL5Mcs2lh1C 9W8akgQ57Sw1Zvl154MisQcWLioF5SBAs6mONZ6WcO4rqEWigNjL7aeDNHlvk0MhwWCE W3ocDkfCsT5ODvigYUn6S1S5W6R62h4bVMVwuYcpuxl7R2bw3ysimHpa2nFRqxxUonVW 5rNOxF6lniQmXPIS4cd3ukAlWZ3TjOhbAwRj4yrwMk5vnRSIlVmxSf8SH0IEk41F5ogS JPQs3gzGY9QMF/RK1H4HW3y7+nTjPTG2HafFNlY7OIN7Jt8+9SqdickWQdDJM5ZF+Zhh M6LQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=MvtoIZiH; spf=pass (google.com: domain of robdclark@gmail.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=robdclark@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=MvtoIZiH; spf=pass (google.com: domain of robdclark@gmail.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=robdclark@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com X-Google-Smtp-Source: AH8x225Up8AdhtAK9u5aTRgYpkaL2HaK1QtQ8n3OW+3Pd+ljgRhbD2lPFy+pCMW/tpkKUetzzDZYfz78EyQPWp2OS3k= 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: Rob Clark Date: Thu, 15 Feb 2018 09:14:40 -0500 Message-ID: Subject: Re: [Freedreno] [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers To: Tomasz Figa 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?1592476650921427048?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Wed, Feb 14, 2018 at 11:09 PM, Tomasz Figa wrote: > 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.) > At least as far as drm/msm/adreno, I'm not terribly concerned about other platforms that don't need to power up iommu. It's not really the same situation as a IP block that shows up in all different vendor's SoCs. But if you can convince Robin to go for get/put_sync() calls inside the iommu driver, I'm fine with that approach too. That is what I do in qcom_iommu already. But if not I'd like to at least solve this for some platforms if we can't solve for all. BR, -R >> >> (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