From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Fri, 03 Mar 2017 18:41:33 +0200 Subject: [RFC PATCH 10/12] staging: android: ion: Use CMA APIs directly In-Reply-To: <1488491084-17252-11-git-send-email-labbott@redhat.com> References: <1488491084-17252-1-git-send-email-labbott@redhat.com> <1488491084-17252-11-git-send-email-labbott@redhat.com> Message-ID: <2140021.hmlAgxcLbU@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Laura, Thank you for the patch. On Thursday 02 Mar 2017 13:44:42 Laura Abbott wrote: > When CMA was first introduced, its primary use was for DMA allocation > and the only way to get CMA memory was to call dma_alloc_coherent. This > put Ion in an awkward position since there was no device structure > readily available and setting one up messed up the coherency model. > These days, CMA can be allocated directly from the APIs. Switch to using > this model to avoid needing a dummy device. This also avoids awkward > caching questions. If the DMA mapping API isn't suitable for today's requirements anymore, I believe that's what needs to be fixed, instead of working around the problem by introducing another use-case-specific API. > Signed-off-by: Laura Abbott > --- > drivers/staging/android/ion/ion_cma_heap.c | 97 +++++++-------------------- > 1 file changed, 26 insertions(+), 71 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_cma_heap.c > b/drivers/staging/android/ion/ion_cma_heap.c index d562fd7..6838825 100644 > --- a/drivers/staging/android/ion/ion_cma_heap.c > +++ b/drivers/staging/android/ion/ion_cma_heap.c > @@ -19,24 +19,19 @@ > #include > #include > #include > -#include > +#include > +#include > > #include "ion.h" > #include "ion_priv.h" > > struct ion_cma_heap { > struct ion_heap heap; > - struct device *dev; > + struct cma *cma; > }; > > #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap) > > -struct ion_cma_buffer_info { > - void *cpu_addr; > - dma_addr_t handle; > - struct sg_table *table; > -}; > - > > /* ION CMA heap operations functions */ > static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer > *buffer, @@ -44,93 +39,53 @@ static int ion_cma_allocate(struct ion_heap > *heap, struct ion_buffer *buffer, unsigned long flags) > { > struct ion_cma_heap *cma_heap = to_cma_heap(heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info; > - > - dev_dbg(dev, "Request buffer allocation len %ld\n", len); > - > - if (buffer->flags & ION_FLAG_CACHED) > - return -EINVAL; > + struct sg_table *table; > + struct page *pages; > + int ret; > > - info = kzalloc(sizeof(*info), GFP_KERNEL); > - if (!info) > + pages = cma_alloc(cma_heap->cma, len, 0); > + if (!pages) > return -ENOMEM; > > - info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle), > - GFP_HIGHUSER | __GFP_ZERO); > - > - if (!info->cpu_addr) { > - dev_err(dev, "Fail to allocate buffer\n"); > + table = kmalloc(sizeof(struct sg_table), GFP_KERNEL); > + if (!table) > goto err; > - } > > - info->table = kmalloc(sizeof(*info->table), GFP_KERNEL); > - if (!info->table) > + ret = sg_alloc_table(table, 1, GFP_KERNEL); > + if (ret) > goto free_mem; > > - if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle, > - len)) > - goto free_table; > - /* keep this for memory release */ > - buffer->priv_virt = info; > - buffer->sg_table = info->table; > - dev_dbg(dev, "Allocate buffer %p\n", buffer); > + sg_set_page(table->sgl, pages, len, 0); > + > + buffer->priv_virt = pages; > + buffer->sg_table = table; > return 0; > > -free_table: > - kfree(info->table); > free_mem: > - dma_free_coherent(dev, len, info->cpu_addr, info->handle); > + kfree(table); > err: > - kfree(info); > + cma_release(cma_heap->cma, pages, buffer->size); > return -ENOMEM; > } > > static void ion_cma_free(struct ion_buffer *buffer) > { > struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > + struct page *pages = buffer->priv_virt; > > - dev_dbg(dev, "Release buffer %p\n", buffer); > /* release memory */ > - dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle); > + cma_release(cma_heap->cma, pages, buffer->size); > /* release sg table */ > - sg_free_table(info->table); > - kfree(info->table); > - kfree(info); > -} > - > -static int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer, > - struct vm_area_struct *vma) > -{ > - struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - > - return dma_mmap_coherent(dev, vma, info->cpu_addr, info->handle, > - buffer->size); > -} > - > -static void *ion_cma_map_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - /* kernel memory mapping has been done at allocation time */ > - return info->cpu_addr; > -} > - > -static void ion_cma_unmap_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > + sg_free_table(buffer->sg_table); > + kfree(buffer->sg_table); > } > > static struct ion_heap_ops ion_cma_ops = { > .allocate = ion_cma_allocate, > .free = ion_cma_free, > - .map_user = ion_cma_mmap, > - .map_kernel = ion_cma_map_kernel, > - .unmap_kernel = ion_cma_unmap_kernel, > + .map_user = ion_heap_map_user, > + .map_kernel = ion_heap_map_kernel, > + .unmap_kernel = ion_heap_unmap_kernel, > }; > > struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) > @@ -147,7 +102,7 @@ struct ion_heap *ion_cma_heap_create(struct > ion_platform_heap *data) * get device from private heaps data, later it > will be > * used to make the link with reserved CMA memory > */ > - cma_heap->dev = data->priv; > + cma_heap->cma = data->priv; > cma_heap->heap.type = ION_HEAP_TYPE_DMA; > return &cma_heap->heap; > } -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC PATCH 10/12] staging: android: ion: Use CMA APIs directly Date: Fri, 03 Mar 2017 18:41:33 +0200 Message-ID: <2140021.hmlAgxcLbU@avalon> References: <1488491084-17252-1-git-send-email-labbott@redhat.com> <1488491084-17252-11-git-send-email-labbott@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3843E6ED7F for ; Fri, 3 Mar 2017 16:41:00 +0000 (UTC) In-Reply-To: <1488491084-17252-11-git-send-email-labbott@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: devel@driverdev.osuosl.org, romlem@google.com, Greg Kroah-Hartman , Riley Andrews , linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-mm@kvack.org, arve@android.com, Mark Brown , Daniel Vetter , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgTGF1cmEsCgpUaGFuayB5b3UgZm9yIHRoZSBwYXRjaC4KCk9uIFRodXJzZGF5IDAyIE1hciAy MDE3IDEzOjQ0OjQyIExhdXJhIEFiYm90dCB3cm90ZToKPiBXaGVuIENNQSB3YXMgZmlyc3QgaW50 cm9kdWNlZCwgaXRzIHByaW1hcnkgdXNlIHdhcyBmb3IgRE1BIGFsbG9jYXRpb24KPiBhbmQgdGhl IG9ubHkgd2F5IHRvIGdldCBDTUEgbWVtb3J5IHdhcyB0byBjYWxsIGRtYV9hbGxvY19jb2hlcmVu dC4gVGhpcwo+IHB1dCBJb24gaW4gYW4gYXdrd2FyZCBwb3NpdGlvbiBzaW5jZSB0aGVyZSB3YXMg bm8gZGV2aWNlIHN0cnVjdHVyZQo+IHJlYWRpbHkgYXZhaWxhYmxlIGFuZCBzZXR0aW5nIG9uZSB1 cCBtZXNzZWQgdXAgdGhlIGNvaGVyZW5jeSBtb2RlbC4KPiBUaGVzZSBkYXlzLCBDTUEgY2FuIGJl IGFsbG9jYXRlZCBkaXJlY3RseSBmcm9tIHRoZSBBUElzLiBTd2l0Y2ggdG8gdXNpbmcKPiB0aGlz IG1vZGVsIHRvIGF2b2lkIG5lZWRpbmcgYSBkdW1teSBkZXZpY2UuIFRoaXMgYWxzbyBhdm9pZHMg YXdrd2FyZAo+IGNhY2hpbmcgcXVlc3Rpb25zLgoKSWYgdGhlIERNQSBtYXBwaW5nIEFQSSBpc24n dCBzdWl0YWJsZSBmb3IgdG9kYXkncyByZXF1aXJlbWVudHMgYW55bW9yZSwgSSAKYmVsaWV2ZSB0 aGF0J3Mgd2hhdCBuZWVkcyB0byBiZSBmaXhlZCwgaW5zdGVhZCBvZiB3b3JraW5nIGFyb3VuZCB0 aGUgcHJvYmxlbSAKYnkgaW50cm9kdWNpbmcgYW5vdGhlciB1c2UtY2FzZS1zcGVjaWZpYyBBUEku Cgo+IFNpZ25lZC1vZmYtYnk6IExhdXJhIEFiYm90dCA8bGFiYm90dEByZWRoYXQuY29tPgo+IC0t LQo+ICBkcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9pb24vaW9uX2NtYV9oZWFwLmMgfCA5NyArKysr KysrLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDI2IGluc2VydGlvbnMo KyksIDcxIGRlbGV0aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvYW5k cm9pZC9pb24vaW9uX2NtYV9oZWFwLmMKPiBiL2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL2lvbi9p b25fY21hX2hlYXAuYyBpbmRleCBkNTYyZmQ3Li42ODM4ODI1IDEwMDY0NAo+IC0tLSBhL2RyaXZl cnMvc3RhZ2luZy9hbmRyb2lkL2lvbi9pb25fY21hX2hlYXAuYwo+ICsrKyBiL2RyaXZlcnMvc3Rh Z2luZy9hbmRyb2lkL2lvbi9pb25fY21hX2hlYXAuYwo+IEBAIC0xOSwyNCArMTksMTkgQEAKPiAg I2luY2x1ZGUgPGxpbnV4L3NsYWIuaD4KPiAgI2luY2x1ZGUgPGxpbnV4L2Vycm5vLmg+Cj4gICNp bmNsdWRlIDxsaW51eC9lcnIuaD4KPiAtI2luY2x1ZGUgPGxpbnV4L2RtYS1tYXBwaW5nLmg+Cj4g KyNpbmNsdWRlIDxsaW51eC9jbWEuaD4KPiArI2luY2x1ZGUgPGxpbnV4L3NjYXR0ZXJsaXN0Lmg+ Cj4gCj4gICNpbmNsdWRlICJpb24uaCIKPiAgI2luY2x1ZGUgImlvbl9wcml2LmgiCj4gCj4gIHN0 cnVjdCBpb25fY21hX2hlYXAgewo+ICAJc3RydWN0IGlvbl9oZWFwIGhlYXA7Cj4gLQlzdHJ1Y3Qg ZGV2aWNlICpkZXY7Cj4gKwlzdHJ1Y3QgY21hICpjbWE7Cj4gIH07Cj4gCj4gICNkZWZpbmUgdG9f Y21hX2hlYXAoeCkgY29udGFpbmVyX29mKHgsIHN0cnVjdCBpb25fY21hX2hlYXAsIGhlYXApCj4g Cj4gLXN0cnVjdCBpb25fY21hX2J1ZmZlcl9pbmZvIHsKPiAtCXZvaWQgKmNwdV9hZGRyOwo+IC0J ZG1hX2FkZHJfdCBoYW5kbGU7Cj4gLQlzdHJ1Y3Qgc2dfdGFibGUgKnRhYmxlOwo+IC19Owo+IC0K PiAKPiAgLyogSU9OIENNQSBoZWFwIG9wZXJhdGlvbnMgZnVuY3Rpb25zICovCj4gIHN0YXRpYyBp bnQgaW9uX2NtYV9hbGxvY2F0ZShzdHJ1Y3QgaW9uX2hlYXAgKmhlYXAsIHN0cnVjdCBpb25fYnVm ZmVyCj4gKmJ1ZmZlciwgQEAgLTQ0LDkzICszOSw1MyBAQCBzdGF0aWMgaW50IGlvbl9jbWFfYWxs b2NhdGUoc3RydWN0IGlvbl9oZWFwCj4gKmhlYXAsIHN0cnVjdCBpb25fYnVmZmVyICpidWZmZXIs IHVuc2lnbmVkIGxvbmcgZmxhZ3MpCj4gIHsKPiAgCXN0cnVjdCBpb25fY21hX2hlYXAgKmNtYV9o ZWFwID0gdG9fY21hX2hlYXAoaGVhcCk7Cj4gLQlzdHJ1Y3QgZGV2aWNlICpkZXYgPSBjbWFfaGVh cC0+ZGV2Owo+IC0Jc3RydWN0IGlvbl9jbWFfYnVmZmVyX2luZm8gKmluZm87Cj4gLQo+IC0JZGV2 X2RiZyhkZXYsICJSZXF1ZXN0IGJ1ZmZlciBhbGxvY2F0aW9uIGxlbiAlbGRcbiIsIGxlbik7Cj4g LQo+IC0JaWYgKGJ1ZmZlci0+ZmxhZ3MgJiBJT05fRkxBR19DQUNIRUQpCj4gLQkJcmV0dXJuIC1F SU5WQUw7Cj4gKwlzdHJ1Y3Qgc2dfdGFibGUgKnRhYmxlOwo+ICsJc3RydWN0IHBhZ2UgKnBhZ2Vz Owo+ICsJaW50IHJldDsKPiAKPiAtCWluZm8gPSBremFsbG9jKHNpemVvZigqaW5mbyksIEdGUF9L RVJORUwpOwo+IC0JaWYgKCFpbmZvKQo+ICsJcGFnZXMgPSBjbWFfYWxsb2MoY21hX2hlYXAtPmNt YSwgbGVuLCAwKTsKPiArCWlmICghcGFnZXMpCj4gIAkJcmV0dXJuIC1FTk9NRU07Cj4gCj4gLQlp bmZvLT5jcHVfYWRkciA9IGRtYV9hbGxvY19jb2hlcmVudChkZXYsIGxlbiwgJihpbmZvLT5oYW5k bGUpLAo+IC0JCQkJCQlHRlBfSElHSFVTRVIgfCBfX0dGUF9aRVJPKTsKPiAtCj4gLQlpZiAoIWlu Zm8tPmNwdV9hZGRyKSB7Cj4gLQkJZGV2X2VycihkZXYsICJGYWlsIHRvIGFsbG9jYXRlIGJ1ZmZl clxuIik7Cj4gKwl0YWJsZSA9IGttYWxsb2Moc2l6ZW9mKHN0cnVjdCBzZ190YWJsZSksIEdGUF9L RVJORUwpOwo+ICsJaWYgKCF0YWJsZSkKPiAgCQlnb3RvIGVycjsKPiAtCX0KPiAKPiAtCWluZm8t PnRhYmxlID0ga21hbGxvYyhzaXplb2YoKmluZm8tPnRhYmxlKSwgR0ZQX0tFUk5FTCk7Cj4gLQlp ZiAoIWluZm8tPnRhYmxlKQo+ICsJcmV0ID0gc2dfYWxsb2NfdGFibGUodGFibGUsIDEsIEdGUF9L RVJORUwpOwo+ICsJaWYgKHJldCkKPiAgCQlnb3RvIGZyZWVfbWVtOwo+IAo+IC0JaWYgKGRtYV9n ZXRfc2d0YWJsZShkZXYsIGluZm8tPnRhYmxlLCBpbmZvLT5jcHVfYWRkciwgaW5mby0+aGFuZGxl LAo+IC0JCQkgICAgbGVuKSkKPiAtCQlnb3RvIGZyZWVfdGFibGU7Cj4gLQkvKiBrZWVwIHRoaXMg Zm9yIG1lbW9yeSByZWxlYXNlICovCj4gLQlidWZmZXItPnByaXZfdmlydCA9IGluZm87Cj4gLQli dWZmZXItPnNnX3RhYmxlID0gaW5mby0+dGFibGU7Cj4gLQlkZXZfZGJnKGRldiwgIkFsbG9jYXRl IGJ1ZmZlciAlcFxuIiwgYnVmZmVyKTsKPiArCXNnX3NldF9wYWdlKHRhYmxlLT5zZ2wsIHBhZ2Vz LCBsZW4sIDApOwo+ICsKPiArCWJ1ZmZlci0+cHJpdl92aXJ0ID0gcGFnZXM7Cj4gKwlidWZmZXIt PnNnX3RhYmxlID0gdGFibGU7Cj4gIAlyZXR1cm4gMDsKPiAKPiAtZnJlZV90YWJsZToKPiAtCWtm cmVlKGluZm8tPnRhYmxlKTsKPiAgZnJlZV9tZW06Cj4gLQlkbWFfZnJlZV9jb2hlcmVudChkZXYs IGxlbiwgaW5mby0+Y3B1X2FkZHIsIGluZm8tPmhhbmRsZSk7Cj4gKwlrZnJlZSh0YWJsZSk7Cj4g IGVycjoKPiAtCWtmcmVlKGluZm8pOwo+ICsJY21hX3JlbGVhc2UoY21hX2hlYXAtPmNtYSwgcGFn ZXMsIGJ1ZmZlci0+c2l6ZSk7Cj4gIAlyZXR1cm4gLUVOT01FTTsKPiAgfQo+IAo+ICBzdGF0aWMg dm9pZCBpb25fY21hX2ZyZWUoc3RydWN0IGlvbl9idWZmZXIgKmJ1ZmZlcikKPiAgewo+ICAJc3Ry dWN0IGlvbl9jbWFfaGVhcCAqY21hX2hlYXAgPSB0b19jbWFfaGVhcChidWZmZXItPmhlYXApOwo+ IC0Jc3RydWN0IGRldmljZSAqZGV2ID0gY21hX2hlYXAtPmRldjsKPiAtCXN0cnVjdCBpb25fY21h X2J1ZmZlcl9pbmZvICppbmZvID0gYnVmZmVyLT5wcml2X3ZpcnQ7Cj4gKwlzdHJ1Y3QgcGFnZSAq cGFnZXMgPSBidWZmZXItPnByaXZfdmlydDsKPiAKPiAtCWRldl9kYmcoZGV2LCAiUmVsZWFzZSBi dWZmZXIgJXBcbiIsIGJ1ZmZlcik7Cj4gIAkvKiByZWxlYXNlIG1lbW9yeSAqLwo+IC0JZG1hX2Zy ZWVfY29oZXJlbnQoZGV2LCBidWZmZXItPnNpemUsIGluZm8tPmNwdV9hZGRyLCBpbmZvLT5oYW5k bGUpOwo+ICsJY21hX3JlbGVhc2UoY21hX2hlYXAtPmNtYSwgcGFnZXMsIGJ1ZmZlci0+c2l6ZSk7 Cj4gIAkvKiByZWxlYXNlIHNnIHRhYmxlICovCj4gLQlzZ19mcmVlX3RhYmxlKGluZm8tPnRhYmxl KTsKPiAtCWtmcmVlKGluZm8tPnRhYmxlKTsKPiAtCWtmcmVlKGluZm8pOwo+IC19Cj4gLQo+IC1z dGF0aWMgaW50IGlvbl9jbWFfbW1hcChzdHJ1Y3QgaW9uX2hlYXAgKm1hcHBlciwgc3RydWN0IGlv bl9idWZmZXIgKmJ1ZmZlciwKPiAtCQkJc3RydWN0IHZtX2FyZWFfc3RydWN0ICp2bWEpCj4gLXsK PiAtCXN0cnVjdCBpb25fY21hX2hlYXAgKmNtYV9oZWFwID0gdG9fY21hX2hlYXAoYnVmZmVyLT5o ZWFwKTsKPiAtCXN0cnVjdCBkZXZpY2UgKmRldiA9IGNtYV9oZWFwLT5kZXY7Cj4gLQlzdHJ1Y3Qg aW9uX2NtYV9idWZmZXJfaW5mbyAqaW5mbyA9IGJ1ZmZlci0+cHJpdl92aXJ0Owo+IC0KPiAtCXJl dHVybiBkbWFfbW1hcF9jb2hlcmVudChkZXYsIHZtYSwgaW5mby0+Y3B1X2FkZHIsIGluZm8tPmhh bmRsZSwKPiAtCQkJCSBidWZmZXItPnNpemUpOwo+IC19Cj4gLQo+IC1zdGF0aWMgdm9pZCAqaW9u X2NtYV9tYXBfa2VybmVsKHN0cnVjdCBpb25faGVhcCAqaGVhcCwKPiAtCQkJCXN0cnVjdCBpb25f YnVmZmVyICpidWZmZXIpCj4gLXsKPiAtCXN0cnVjdCBpb25fY21hX2J1ZmZlcl9pbmZvICppbmZv ID0gYnVmZmVyLT5wcml2X3ZpcnQ7Cj4gLQkvKiBrZXJuZWwgbWVtb3J5IG1hcHBpbmcgaGFzIGJl ZW4gZG9uZSBhdCBhbGxvY2F0aW9uIHRpbWUgKi8KPiAtCXJldHVybiBpbmZvLT5jcHVfYWRkcjsK PiAtfQo+IC0KPiAtc3RhdGljIHZvaWQgaW9uX2NtYV91bm1hcF9rZXJuZWwoc3RydWN0IGlvbl9o ZWFwICpoZWFwLAo+IC0JCQkJIHN0cnVjdCBpb25fYnVmZmVyICpidWZmZXIpCj4gLXsKPiArCXNn X2ZyZWVfdGFibGUoYnVmZmVyLT5zZ190YWJsZSk7Cj4gKwlrZnJlZShidWZmZXItPnNnX3RhYmxl KTsKPiAgfQo+IAo+ICBzdGF0aWMgc3RydWN0IGlvbl9oZWFwX29wcyBpb25fY21hX29wcyA9IHsK PiAgCS5hbGxvY2F0ZSA9IGlvbl9jbWFfYWxsb2NhdGUsCj4gIAkuZnJlZSA9IGlvbl9jbWFfZnJl ZSwKPiAtCS5tYXBfdXNlciA9IGlvbl9jbWFfbW1hcCwKPiAtCS5tYXBfa2VybmVsID0gaW9uX2Nt YV9tYXBfa2VybmVsLAo+IC0JLnVubWFwX2tlcm5lbCA9IGlvbl9jbWFfdW5tYXBfa2VybmVsLAo+ ICsJLm1hcF91c2VyID0gaW9uX2hlYXBfbWFwX3VzZXIsCj4gKwkubWFwX2tlcm5lbCA9IGlvbl9o ZWFwX21hcF9rZXJuZWwsCj4gKwkudW5tYXBfa2VybmVsID0gaW9uX2hlYXBfdW5tYXBfa2VybmVs LAo+ICB9Owo+IAo+ICBzdHJ1Y3QgaW9uX2hlYXAgKmlvbl9jbWFfaGVhcF9jcmVhdGUoc3RydWN0 IGlvbl9wbGF0Zm9ybV9oZWFwICpkYXRhKQo+IEBAIC0xNDcsNyArMTAyLDcgQEAgc3RydWN0IGlv bl9oZWFwICppb25fY21hX2hlYXBfY3JlYXRlKHN0cnVjdAo+IGlvbl9wbGF0Zm9ybV9oZWFwICpk YXRhKSAqIGdldCBkZXZpY2UgZnJvbSBwcml2YXRlIGhlYXBzIGRhdGEsIGxhdGVyIGl0Cj4gd2ls bCBiZQo+ICAJICogdXNlZCB0byBtYWtlIHRoZSBsaW5rIHdpdGggcmVzZXJ2ZWQgQ01BIG1lbW9y eQo+ICAJICovCj4gLQljbWFfaGVhcC0+ZGV2ID0gZGF0YS0+cHJpdjsKPiArCWNtYV9oZWFwLT5j bWEgPSBkYXRhLT5wcml2Owo+ICAJY21hX2hlYXAtPmhlYXAudHlwZSA9IElPTl9IRUFQX1RZUEVf RE1BOwo+ICAJcmV0dXJuICZjbWFfaGVhcC0+aGVhcDsKPiAgfQoKLS0gClJlZ2FyZHMsCgpMYXVy ZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2 ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from galahad.ideasonboard.com ([185.26.127.97]:40455 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751459AbdCCQls (ORCPT ); Fri, 3 Mar 2017 11:41:48 -0500 From: Laurent Pinchart To: dri-devel@lists.freedesktop.org Cc: Laura Abbott , Sumit Semwal , Riley Andrews , arve@android.com, devel@driverdev.osuosl.org, romlem@google.com, Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-mm@kvack.org, Mark Brown , Daniel Vetter , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Subject: Re: [RFC PATCH 10/12] staging: android: ion: Use CMA APIs directly Date: Fri, 03 Mar 2017 18:41:33 +0200 Message-ID: <2140021.hmlAgxcLbU@avalon> In-Reply-To: <1488491084-17252-11-git-send-email-labbott@redhat.com> References: <1488491084-17252-1-git-send-email-labbott@redhat.com> <1488491084-17252-11-git-send-email-labbott@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laura, Thank you for the patch. On Thursday 02 Mar 2017 13:44:42 Laura Abbott wrote: > When CMA was first introduced, its primary use was for DMA allocation > and the only way to get CMA memory was to call dma_alloc_coherent. This > put Ion in an awkward position since there was no device structure > readily available and setting one up messed up the coherency model. > These days, CMA can be allocated directly from the APIs. Switch to using > this model to avoid needing a dummy device. This also avoids awkward > caching questions. If the DMA mapping API isn't suitable for today's requirements anymore, I believe that's what needs to be fixed, instead of working around the problem by introducing another use-case-specific API. > Signed-off-by: Laura Abbott > --- > drivers/staging/android/ion/ion_cma_heap.c | 97 +++++++-------------------- > 1 file changed, 26 insertions(+), 71 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_cma_heap.c > b/drivers/staging/android/ion/ion_cma_heap.c index d562fd7..6838825 100644 > --- a/drivers/staging/android/ion/ion_cma_heap.c > +++ b/drivers/staging/android/ion/ion_cma_heap.c > @@ -19,24 +19,19 @@ > #include > #include > #include > -#include > +#include > +#include > > #include "ion.h" > #include "ion_priv.h" > > struct ion_cma_heap { > struct ion_heap heap; > - struct device *dev; > + struct cma *cma; > }; > > #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap) > > -struct ion_cma_buffer_info { > - void *cpu_addr; > - dma_addr_t handle; > - struct sg_table *table; > -}; > - > > /* ION CMA heap operations functions */ > static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer > *buffer, @@ -44,93 +39,53 @@ static int ion_cma_allocate(struct ion_heap > *heap, struct ion_buffer *buffer, unsigned long flags) > { > struct ion_cma_heap *cma_heap = to_cma_heap(heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info; > - > - dev_dbg(dev, "Request buffer allocation len %ld\n", len); > - > - if (buffer->flags & ION_FLAG_CACHED) > - return -EINVAL; > + struct sg_table *table; > + struct page *pages; > + int ret; > > - info = kzalloc(sizeof(*info), GFP_KERNEL); > - if (!info) > + pages = cma_alloc(cma_heap->cma, len, 0); > + if (!pages) > return -ENOMEM; > > - info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle), > - GFP_HIGHUSER | __GFP_ZERO); > - > - if (!info->cpu_addr) { > - dev_err(dev, "Fail to allocate buffer\n"); > + table = kmalloc(sizeof(struct sg_table), GFP_KERNEL); > + if (!table) > goto err; > - } > > - info->table = kmalloc(sizeof(*info->table), GFP_KERNEL); > - if (!info->table) > + ret = sg_alloc_table(table, 1, GFP_KERNEL); > + if (ret) > goto free_mem; > > - if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle, > - len)) > - goto free_table; > - /* keep this for memory release */ > - buffer->priv_virt = info; > - buffer->sg_table = info->table; > - dev_dbg(dev, "Allocate buffer %p\n", buffer); > + sg_set_page(table->sgl, pages, len, 0); > + > + buffer->priv_virt = pages; > + buffer->sg_table = table; > return 0; > > -free_table: > - kfree(info->table); > free_mem: > - dma_free_coherent(dev, len, info->cpu_addr, info->handle); > + kfree(table); > err: > - kfree(info); > + cma_release(cma_heap->cma, pages, buffer->size); > return -ENOMEM; > } > > static void ion_cma_free(struct ion_buffer *buffer) > { > struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > + struct page *pages = buffer->priv_virt; > > - dev_dbg(dev, "Release buffer %p\n", buffer); > /* release memory */ > - dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle); > + cma_release(cma_heap->cma, pages, buffer->size); > /* release sg table */ > - sg_free_table(info->table); > - kfree(info->table); > - kfree(info); > -} > - > -static int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer, > - struct vm_area_struct *vma) > -{ > - struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - > - return dma_mmap_coherent(dev, vma, info->cpu_addr, info->handle, > - buffer->size); > -} > - > -static void *ion_cma_map_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - /* kernel memory mapping has been done at allocation time */ > - return info->cpu_addr; > -} > - > -static void ion_cma_unmap_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > + sg_free_table(buffer->sg_table); > + kfree(buffer->sg_table); > } > > static struct ion_heap_ops ion_cma_ops = { > .allocate = ion_cma_allocate, > .free = ion_cma_free, > - .map_user = ion_cma_mmap, > - .map_kernel = ion_cma_map_kernel, > - .unmap_kernel = ion_cma_unmap_kernel, > + .map_user = ion_heap_map_user, > + .map_kernel = ion_heap_map_kernel, > + .unmap_kernel = ion_heap_unmap_kernel, > }; > > struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) > @@ -147,7 +102,7 @@ struct ion_heap *ion_cma_heap_create(struct > ion_platform_heap *data) * get device from private heaps data, later it > will be > * used to make the link with reserved CMA memory > */ > - cma_heap->dev = data->priv; > + cma_heap->cma = data->priv; > cma_heap->heap.type = ION_HEAP_TYPE_DMA; > return &cma_heap->heap; > } -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f71.google.com (mail-wm0-f71.google.com [74.125.82.71]) by kanga.kvack.org (Postfix) with ESMTP id 9B91C6B0038 for ; Fri, 3 Mar 2017 11:41:00 -0500 (EST) Received: by mail-wm0-f71.google.com with SMTP id m70so7738668wma.2 for ; Fri, 03 Mar 2017 08:41:00 -0800 (PST) Received: from galahad.ideasonboard.com (galahad.ideasonboard.com. [185.26.127.97]) by mx.google.com with ESMTPS id n54si15471531wrn.247.2017.03.03.08.40.59 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Mar 2017 08:40:59 -0800 (PST) From: Laurent Pinchart Subject: Re: [RFC PATCH 10/12] staging: android: ion: Use CMA APIs directly Date: Fri, 03 Mar 2017 18:41:33 +0200 Message-ID: <2140021.hmlAgxcLbU@avalon> In-Reply-To: <1488491084-17252-11-git-send-email-labbott@redhat.com> References: <1488491084-17252-1-git-send-email-labbott@redhat.com> <1488491084-17252-11-git-send-email-labbott@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: owner-linux-mm@kvack.org List-ID: To: dri-devel@lists.freedesktop.org Cc: Laura Abbott , Sumit Semwal , Riley Andrews , arve@android.com, devel@driverdev.osuosl.org, romlem@google.com, Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-mm@kvack.org, Mark Brown , Daniel Vetter , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Hi Laura, Thank you for the patch. On Thursday 02 Mar 2017 13:44:42 Laura Abbott wrote: > When CMA was first introduced, its primary use was for DMA allocation > and the only way to get CMA memory was to call dma_alloc_coherent. This > put Ion in an awkward position since there was no device structure > readily available and setting one up messed up the coherency model. > These days, CMA can be allocated directly from the APIs. Switch to using > this model to avoid needing a dummy device. This also avoids awkward > caching questions. If the DMA mapping API isn't suitable for today's requirements anymore, I believe that's what needs to be fixed, instead of working around the problem by introducing another use-case-specific API. > Signed-off-by: Laura Abbott > --- > drivers/staging/android/ion/ion_cma_heap.c | 97 +++++++-------------------- > 1 file changed, 26 insertions(+), 71 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_cma_heap.c > b/drivers/staging/android/ion/ion_cma_heap.c index d562fd7..6838825 100644 > --- a/drivers/staging/android/ion/ion_cma_heap.c > +++ b/drivers/staging/android/ion/ion_cma_heap.c > @@ -19,24 +19,19 @@ > #include > #include > #include > -#include > +#include > +#include > > #include "ion.h" > #include "ion_priv.h" > > struct ion_cma_heap { > struct ion_heap heap; > - struct device *dev; > + struct cma *cma; > }; > > #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap) > > -struct ion_cma_buffer_info { > - void *cpu_addr; > - dma_addr_t handle; > - struct sg_table *table; > -}; > - > > /* ION CMA heap operations functions */ > static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer > *buffer, @@ -44,93 +39,53 @@ static int ion_cma_allocate(struct ion_heap > *heap, struct ion_buffer *buffer, unsigned long flags) > { > struct ion_cma_heap *cma_heap = to_cma_heap(heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info; > - > - dev_dbg(dev, "Request buffer allocation len %ld\n", len); > - > - if (buffer->flags & ION_FLAG_CACHED) > - return -EINVAL; > + struct sg_table *table; > + struct page *pages; > + int ret; > > - info = kzalloc(sizeof(*info), GFP_KERNEL); > - if (!info) > + pages = cma_alloc(cma_heap->cma, len, 0); > + if (!pages) > return -ENOMEM; > > - info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle), > - GFP_HIGHUSER | __GFP_ZERO); > - > - if (!info->cpu_addr) { > - dev_err(dev, "Fail to allocate buffer\n"); > + table = kmalloc(sizeof(struct sg_table), GFP_KERNEL); > + if (!table) > goto err; > - } > > - info->table = kmalloc(sizeof(*info->table), GFP_KERNEL); > - if (!info->table) > + ret = sg_alloc_table(table, 1, GFP_KERNEL); > + if (ret) > goto free_mem; > > - if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle, > - len)) > - goto free_table; > - /* keep this for memory release */ > - buffer->priv_virt = info; > - buffer->sg_table = info->table; > - dev_dbg(dev, "Allocate buffer %p\n", buffer); > + sg_set_page(table->sgl, pages, len, 0); > + > + buffer->priv_virt = pages; > + buffer->sg_table = table; > return 0; > > -free_table: > - kfree(info->table); > free_mem: > - dma_free_coherent(dev, len, info->cpu_addr, info->handle); > + kfree(table); > err: > - kfree(info); > + cma_release(cma_heap->cma, pages, buffer->size); > return -ENOMEM; > } > > static void ion_cma_free(struct ion_buffer *buffer) > { > struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > + struct page *pages = buffer->priv_virt; > > - dev_dbg(dev, "Release buffer %p\n", buffer); > /* release memory */ > - dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle); > + cma_release(cma_heap->cma, pages, buffer->size); > /* release sg table */ > - sg_free_table(info->table); > - kfree(info->table); > - kfree(info); > -} > - > -static int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer, > - struct vm_area_struct *vma) > -{ > - struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap); > - struct device *dev = cma_heap->dev; > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - > - return dma_mmap_coherent(dev, vma, info->cpu_addr, info->handle, > - buffer->size); > -} > - > -static void *ion_cma_map_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > - struct ion_cma_buffer_info *info = buffer->priv_virt; > - /* kernel memory mapping has been done at allocation time */ > - return info->cpu_addr; > -} > - > -static void ion_cma_unmap_kernel(struct ion_heap *heap, > - struct ion_buffer *buffer) > -{ > + sg_free_table(buffer->sg_table); > + kfree(buffer->sg_table); > } > > static struct ion_heap_ops ion_cma_ops = { > .allocate = ion_cma_allocate, > .free = ion_cma_free, > - .map_user = ion_cma_mmap, > - .map_kernel = ion_cma_map_kernel, > - .unmap_kernel = ion_cma_unmap_kernel, > + .map_user = ion_heap_map_user, > + .map_kernel = ion_heap_map_kernel, > + .unmap_kernel = ion_heap_unmap_kernel, > }; > > struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) > @@ -147,7 +102,7 @@ struct ion_heap *ion_cma_heap_create(struct > ion_platform_heap *data) * get device from private heaps data, later it > will be > * used to make the link with reserved CMA memory > */ > - cma_heap->dev = data->priv; > + cma_heap->cma = data->priv; > cma_heap->heap.type = ION_HEAP_TYPE_DMA; > return &cma_heap->heap; > } -- Regards, Laurent Pinchart -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org