From mboxrd@z Thu Jan 1 00:00:00 1970 From: Inki Dae Subject: Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access Date: Mon, 19 Dec 2016 10:40:41 +0900 Message-ID: <58573A99.2050809@samsung.com> References: <1466492640-12551-1-git-send-email-chris@chris-wilson.co.uk> <1471275738-31994-1-git-send-email-chris@chris-wilson.co.uk> <20160815160214.GK6232@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <20160815160214.GK6232@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Sumit Semwal , Eric Anholt , linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org CgoyMDE264WEIDA47JuUIDE27J28IDAxOjAy7JeQIERhbmllbCBWZXR0ZXIg7J20KOqwgCkg7JO0 IOq4gDoKPiBPbiBNb24sIEF1ZyAxNSwgMjAxNiBhdCAwNDo0MjoxOFBNICswMTAwLCBDaHJpcyBX aWxzb24gd3JvdGU6Cj4+IFJlbmRlcmluZyBvcGVyYXRpb25zIHRvIHRoZSBkbWEtYnVmIGFyZSB0 cmFja2VkIGltcGxpY2l0bHkgdmlhIHRoZQo+PiByZXNlcnZhdGlvbl9vYmplY3QgKGRtYWJ1Zi0+ cmVzdikuIFRoaXMgaXMgdXNlZCB0byBhbGxvdyBwb2xsKCkgdG8KPj4gd2FpdCB1cG9uIG91dHN0 YW5kaW5nIHJlbmRlcmluZyAob3IganVzdCBxdWVyeSB0aGUgY3VycmVudCBzdGF0dXMgb2YKPj4g cmVuZGVyaW5nKS4gVGhlIGRtYS1idWYgc3luYyBpb2N0bCBhbGxvd3MgdXNlcnNwYWNlIHRvIHBy ZXBhcmUgdGhlCj4+IGRtYS1idWYgZm9yIENQVSBhY2Nlc3MsIHdoaWNoIHNob3VsZCBpbmNsdWRl IHdhaXRpbmcgdXBvbiByZW5kZXJpbmcuCj4+IChTb21lIGRyaXZlcnMgbWF5IG5lZWQgdG8gZG8g bW9yZSB3b3JrIHRvIGVuc3VyZSB0aGF0IHRoZSBkbWEtYnVmIG1tYXAKPj4gaXMgY29oZXJlbnQg YXMgd2VsbCBhcyBjb21wbGV0ZS4pCj4+Cj4+IHYyOiBBbHdheXMgd2FpdCB1cG9uIHRoZSByZXNl cnZhdGlvbiBvYmplY3QgaW1wbGljaXRseS4gV2UgY2hvb3NlIHRvIGRvCj4+IGl0IGFmdGVyIHRo ZSBuYXRpdmUgaGFuZGxlciBpbiBjYXNlIGl0IGNhbiBkbyBzbyBtb3JlIGVmZmljaWVudGx5Lgo+ Pgo+PiBUZXN0Y2FzZTogaWd0L3ByaW1lX3ZnZW0KPj4gVGVzdGNhc2U6IGlndC9nZW1fY29uY3Vy cmVudF9ibGl0ICMgKnZnZW0qCj4+IFNpZ25lZC1vZmYtYnk6IENocmlzIFdpbHNvbiA8Y2hyaXNA Y2hyaXMtd2lsc29uLmNvLnVrPgo+PiBDYzogU3VtaXQgU2Vtd2FsIDxzdW1pdC5zZW13YWxAbGlu YXJvLm9yZz4KPj4gQ2M6IERhbmllbCBWZXR0ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4+ IENjOiBFcmljIEFuaG9sdCA8ZXJpY0BhbmhvbHQubmV0Pgo+PiBDYzogbGludXgtbWVkaWFAdmdl ci5rZXJuZWwub3JnCj4+IENjOiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4+IENj OiBsaW5hcm8tbW0tc2lnQGxpc3RzLmxpbmFyby5vcmcKPj4gQ2M6IGxpbnV4LWtlcm5lbEB2Z2Vy Lmtlcm5lbC5vcmcKPj4gLS0tCj4+ICBkcml2ZXJzL2RtYS1idWYvZG1hLWJ1Zi5jIHwgMjMgKysr KysrKysrKysrKysrKysrKysrKysKPj4gIDEgZmlsZSBjaGFuZ2VkLCAyMyBpbnNlcnRpb25zKCsp Cj4+Cj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2RtYS1idWYvZG1hLWJ1Zi5jIGIvZHJpdmVycy9k bWEtYnVmL2RtYS1idWYuYwo+PiBpbmRleCBkZGFlZTYwYWU1MmEuLmNmMDRkMjQ5YTZhNCAxMDA2 NDQKPj4gLS0tIGEvZHJpdmVycy9kbWEtYnVmL2RtYS1idWYuYwo+PiArKysgYi9kcml2ZXJzL2Rt YS1idWYvZG1hLWJ1Zi5jCj4+IEBAIC01ODYsNiArNTg2LDIyIEBAIHZvaWQgZG1hX2J1Zl91bm1h cF9hdHRhY2htZW50KHN0cnVjdCBkbWFfYnVmX2F0dGFjaG1lbnQgKmF0dGFjaCwKPj4gIH0KPj4g IEVYUE9SVF9TWU1CT0xfR1BMKGRtYV9idWZfdW5tYXBfYXR0YWNobWVudCk7Cj4+ICAKPj4gK3N0 YXRpYyBpbnQgX19kbWFfYnVmX2JlZ2luX2NwdV9hY2Nlc3Moc3RydWN0IGRtYV9idWYgKmRtYWJ1 ZiwKPj4gKwkJCQkgICAgICBlbnVtIGRtYV9kYXRhX2RpcmVjdGlvbiBkaXJlY3Rpb24pCj4+ICt7 Cj4+ICsJYm9vbCB3cml0ZSA9IChkaXJlY3Rpb24gPT0gRE1BX0JJRElSRUNUSU9OQUwgfHwKPj4g KwkJICAgICAgZGlyZWN0aW9uID09IERNQV9UT19ERVZJQ0UpOwo+PiArCXN0cnVjdCByZXNlcnZh dGlvbl9vYmplY3QgKnJlc3YgPSBkbWFidWYtPnJlc3Y7Cj4+ICsJbG9uZyByZXQ7Cj4+ICsKPj4g KwkvKiBXYWl0IG9uIGFueSBpbXBsaWNpdCByZW5kZXJpbmcgZmVuY2VzICovCj4+ICsJcmV0ID0g cmVzZXJ2YXRpb25fb2JqZWN0X3dhaXRfdGltZW91dF9yY3UocmVzdiwgd3JpdGUsIHRydWUsCj4+ ICsJCQkJCQkgIE1BWF9TQ0hFRFVMRV9USU1FT1VUKTsKPj4gKwlpZiAocmV0IDwgMCkKPj4gKwkJ cmV0dXJuIHJldDsKPj4gKwo+PiArCXJldHVybiAwOwo+PiArfQo+PiAgCj4+ICAvKioKPj4gICAq IGRtYV9idWZfYmVnaW5fY3B1X2FjY2VzcyAtIE11c3QgYmUgY2FsbGVkIGJlZm9yZSBhY2Nlc3Np bmcgYSBkbWFfYnVmIGZyb20gdGhlCj4+IEBAIC02MDgsNiArNjI0LDEzIEBAIGludCBkbWFfYnVm X2JlZ2luX2NwdV9hY2Nlc3Moc3RydWN0IGRtYV9idWYgKmRtYWJ1ZiwKPj4gIAlpZiAoZG1hYnVm LT5vcHMtPmJlZ2luX2NwdV9hY2Nlc3MpCj4+ICAJCXJldCA9IGRtYWJ1Zi0+b3BzLT5iZWdpbl9j cHVfYWNjZXNzKGRtYWJ1ZiwgZGlyZWN0aW9uKTsKPj4gIAo+PiArCS8qIEVuc3VyZSB0aGF0IGFs bCBmZW5jZXMgYXJlIHdhaXRlZCB1cG9uIC0gYnV0IHdlIGZpcnN0IGFsbG93Cj4+ICsJICogdGhl IG5hdGl2ZSBoYW5kbGVyIHRoZSBjaGFuY2UgdG8gZG8gc28gbW9yZSBlZmZpY2llbnRseSBpZiBp dAo+PiArCSAqIGNob29zZXMuIEEgZG91YmxlIGludm9jYXRpb24gaGVyZSB3aWxsIGJlIHJlYXNv bmFibHkgY2hlYXAgbm8tb3AuCj4+ICsJICovCj4+ICsJaWYgKHJldCA9PSAwKQo+PiArCQlyZXQg PSBfX2RtYV9idWZfYmVnaW5fY3B1X2FjY2VzcyhkbWFidWYsIGRpcmVjdGlvbik7Cj4gCj4gTm90 IHN1cmUgd2Ugc2hvdWxkIHdhaXQgZmlyc3QgYW5kIHRoZSBmbHVzaCBvciB0aGUgb3RoZXIgd2F5 IHJvdW5kLiBCdXQgSQo+IGRvbid0IHRoaW5rIGl0J2xsIG1hdHRlciBmb3IgYW55IGN1cnJlbnQg ZG1hLWJ1ZiBleHBvcnRlciwgc28gbWVoLgo+IAoKU29ycnkgZm9yIGxhdGUgY29tbWVudC4gSSB3 b25kZXIgdGhlcmUgaXMgbm8gcHJvYmxlbSBpbiBjYXNlIHRoYXQgR1BVIG9yIG90aGVyIERNQSBk ZXZpY2UgdHJpZXMgdG8gYWNjZXNzIHRoaXMgZG1hIGJ1ZmZlciBhZnRlciBkbWFfYnVmX2JlZ2lu X2NwdV9hY2Nlc3MgY2FsbC4KSSB0aGluayBpbiB0aGlzIGNhc2UsIHRoZXkgLSBHUFUgb3IgRE1B IGRldmljZXMgLSB3b3VsZCBtYWtlIGEgbWVzcyBvZiB0aGUgZG1hIGJ1ZmZlciB3aGlsZSBDUFUg aXMgYWNjZXNzaW5nIHRoZSBidWZmZXIuCgpUaGlzIHBhdGNoIGlzIGluIG1haW5saW5lIGFscmVh ZHkgc28gaWYgdGhpcyBpcyByZWFsIHByb2JsZW0gdGhlbiBJIHRoaW5rIHdlIHNvdWxkIGNob29z ZSwKMS4gcmV2ZXJ0IHRoaXMgcGF0Y2ggZnJvbSBtYWlubGluZQoyLiBtYWtlIHN1cmUgdG8gcHJl dmVudCBvdGhlciBETUEgZGV2aWNlcyB0byB0cnkgdG8gYWNjZXNzIHRoZSBidWZmZXIgd2hpbGUg Q1BVIGlzIGFjY2Vzc2luZyB0aGUgYnVmZmVyLgoKVGhhbmtzLgoKPiBSZXZpZXdlZC1ieTogRGFu aWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPiAKPiBTdW1pdHMsIGNhbiB5b3Ug cGxzIHBpY2sgdGhpcyBvbmUgdXAgYW5kIHB1dCBpbnRvIGRybS1taXNjPwo+IC1EYW5pZWwKPiAK Pj4gKwo+PiAgCXJldHVybiByZXQ7Cj4+ICB9Cj4+ICBFWFBPUlRfU1lNQk9MX0dQTChkbWFfYnVm X2JlZ2luX2NwdV9hY2Nlc3MpOwo+PiAtLSAKPj4gMi44LjEKPj4KPiAKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApk cmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mailout4.samsung.com ([203.254.224.34]:41504 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750899AbcLSBko (ORCPT ); Sun, 18 Dec 2016 20:40:44 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 8BIT Message-id: <58573A99.2050809@samsung.com> Date: Mon, 19 Dec 2016 10:40:41 +0900 From: Inki Dae To: Chris Wilson , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Sumit Semwal , Eric Anholt , linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access References: <1466492640-12551-1-git-send-email-chris@chris-wilson.co.uk> <1471275738-31994-1-git-send-email-chris@chris-wilson.co.uk> <20160815160214.GK6232@phenom.ffwll.local> In-reply-to: <20160815160214.GK6232@phenom.ffwll.local> Sender: linux-media-owner@vger.kernel.org List-ID: 2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글: > On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote: >> Rendering operations to the dma-buf are tracked implicitly via the >> reservation_object (dmabuf->resv). This is used to allow poll() to >> wait upon outstanding rendering (or just query the current status of >> rendering). The dma-buf sync ioctl allows userspace to prepare the >> dma-buf for CPU access, which should include waiting upon rendering. >> (Some drivers may need to do more work to ensure that the dma-buf mmap >> is coherent as well as complete.) >> >> v2: Always wait upon the reservation object implicitly. We choose to do >> it after the native handler in case it can do so more efficiently. >> >> Testcase: igt/prime_vgem >> Testcase: igt/gem_concurrent_blit # *vgem* >> Signed-off-by: Chris Wilson >> Cc: Sumit Semwal >> Cc: Daniel Vetter >> Cc: Eric Anholt >> Cc: linux-media@vger.kernel.org >> Cc: dri-devel@lists.freedesktop.org >> Cc: linaro-mm-sig@lists.linaro.org >> Cc: linux-kernel@vger.kernel.org >> --- >> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >> index ddaee60ae52a..cf04d249a6a4 100644 >> --- a/drivers/dma-buf/dma-buf.c >> +++ b/drivers/dma-buf/dma-buf.c >> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, >> } >> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment); >> >> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf, >> + enum dma_data_direction direction) >> +{ >> + bool write = (direction == DMA_BIDIRECTIONAL || >> + direction == DMA_TO_DEVICE); >> + struct reservation_object *resv = dmabuf->resv; >> + long ret; >> + >> + /* Wait on any implicit rendering fences */ >> + ret = reservation_object_wait_timeout_rcu(resv, write, true, >> + MAX_SCHEDULE_TIMEOUT); >> + if (ret < 0) >> + return ret; >> + >> + return 0; >> +} >> >> /** >> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the >> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf, >> if (dmabuf->ops->begin_cpu_access) >> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction); >> >> + /* Ensure that all fences are waited upon - but we first allow >> + * the native handler the chance to do so more efficiently if it >> + * chooses. A double invocation here will be reasonably cheap no-op. >> + */ >> + if (ret == 0) >> + ret = __dma_buf_begin_cpu_access(dmabuf, direction); > > Not sure we should wait first and the flush or the other way round. But I > don't think it'll matter for any current dma-buf exporter, so meh. > Sorry for late comment. I wonder there is no problem in case that GPU or other DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call. I think in this case, they - GPU or DMA devices - would make a mess of the dma buffer while CPU is accessing the buffer. This patch is in mainline already so if this is real problem then I think we sould choose, 1. revert this patch from mainline 2. make sure to prevent other DMA devices to try to access the buffer while CPU is accessing the buffer. Thanks. > Reviewed-by: Daniel Vetter > > Sumits, can you pls pick this one up and put into drm-misc? > -Daniel > >> + >> return ret; >> } >> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access); >> -- >> 2.8.1 >> >