From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Thompson Subject: Re: [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible Date: Tue, 03 Feb 2015 09:29:21 +0000 Message-ID: <54D094F1.50404@linaro.org> References: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sumit Semwal , linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-arm-kernel@lists.infradead.org, rmk+kernel@arm.linux.org.uk, airlied@linux.ie, kgene@kernel.org, daniel.vetter@intel.com, thierry.reding@gmail.com, pawel@osciak.com, m.szyprowski@samsung.com, mchehab@osg.samsung.com, gregkh@linuxfoundation.org Cc: linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, linaro-kernel@lists.linaro.org List-Id: linux-tegra@vger.kernel.org T24gMjgvMDEvMTUgMTI6NTQsIFN1bWl0IFNlbXdhbCB3cm90ZToKPiBBdCBwcmVzZW50LCBkbWFf YnVmX2V4cG9ydCgpIHRha2VzIGEgc2VyaWVzIG9mIHBhcmFtZXRlcnMsIHdoaWNoCj4gbWFrZXMg aXQgZGlmZmljdWx0IHRvIGFkZCBhbnkgbmV3IHBhcmFtZXRlcnMgZm9yIGV4cG9ydGVycywgaWYg cmVxdWlyZWQuCj4gCj4gTWFrZSBpdCBzaW1wbGVyIGJ5IG1vdmluZyBhbGwgdGhlc2UgcGFyYW1l dGVycyBpbnRvIGEgc3RydWN0LCBhbmQgcGFzcwo+IHRoZSBzdHJ1Y3QgKiBhcyBwYXJhbWV0ZXIg dG8gZG1hX2J1Zl9leHBvcnQoKS4KPiAKPiBXaGlsZSBhdCBpdCwgdW5pdGUgZG1hX2J1Zl9leHBv cnRfbmFtZWQoKSB3aXRoIGRtYV9idWZfZXhwb3J0KCksIGFuZAo+IGNoYW5nZSBhbGwgY2FsbGVy cyBhY2NvcmRpbmdseS4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBTdW1pdCBTZW13YWwgPHN1bWl0LnNl bXdhbEBsaW5hcm8ub3JnPgoKU29ycnksIGEgZmV3IG1vcmUgY29tbWVudHMuIFNob3VsZCBoYXZl IHNlbnQgdGhlc2UgYmVmb3JlIGJ1dCBhdCBsZWFzdAp0aGUgYXJlIGFsbCByZWxhdGVkIG9ubHkg dG8gZG9jdW1lbnRhdGlvbi4gT25jZSB0aGF0IGlzIGZpeGVkIHRoZW46ClJldmlld2VkLWJ5OiBE YW5pZWwgVGhvbXBzb24gPGRhbmllbC50aG9tcHNvbkBsaW5hcm8ub3JnPgoKCj4gLS0tCj4gdjM6 IERhbmllbCBUaG9tcHNvbiBjYXVnaHQgdGhlIEM5OSB3YXJuaW5nIGlzc3VlIHcvIHVzaW5nIHsw fTsgdXNpbmcKPiAgICAgey5leHBfbmFtZSA9IHh4eH0gaW5zdGVhZC4KPiAKPiB2MjogYWRkIG1h Y3JvIHRvIHplcm8gb3V0IGxvY2FsIHN0cnVjdCwgYW5kIGZpbGwgS0JVSUxEX01PRE5BTUUgYnkg ZGVmYXVsdAo+IAo+ICBkcml2ZXJzL2RtYS1idWYvZG1hLWJ1Zi5jICAgICAgICAgICAgICAgICAg ICAgIHwgNDcgKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0KPiAgZHJpdmVycy9ncHUvZHJtL2Fy bWFkYS9hcm1hZGFfZ2VtLmMgICAgICAgICAgICB8IDEwICsrKystLQo+ICBkcml2ZXJzL2dwdS9k cm0vZHJtX3ByaW1lLmMgICAgICAgICAgICAgICAgICAgIHwgMTIgKysrKy0tLQo+ICBkcml2ZXJz L2dwdS9kcm0vZXh5bm9zL2V4eW5vc19kcm1fZG1hYnVmLmMgICAgIHwgIDkgKysrLS0KPiAgZHJp dmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fZG1hYnVmLmMgICAgICAgICB8IDEwICsrKystLQo+ ICBkcml2ZXJzL2dwdS9kcm0vb21hcGRybS9vbWFwX2dlbV9kbWFidWYuYyAgICAgIHwgIDkgKysr Ky0KPiAgZHJpdmVycy9ncHUvZHJtL3RlZ3JhL2dlbS5jICAgICAgICAgICAgICAgICAgICB8IDEw ICsrKystLQo+ICBkcml2ZXJzL2dwdS9kcm0vdHRtL3R0bV9vYmplY3QuYyAgICAgICAgICAgICAg IHwgIDkgKysrLS0KPiAgZHJpdmVycy9ncHUvZHJtL3VkbC91ZGxfZG1hYnVmLmMgICAgICAgICAg ICAgICB8ICA5ICsrKystCj4gIGRyaXZlcnMvbWVkaWEvdjRsMi1jb3JlL3ZpZGVvYnVmMi1kbWEt Y29udGlnLmMgfCAgOCArKysrLQo+ICBkcml2ZXJzL21lZGlhL3Y0bDItY29yZS92aWRlb2J1ZjIt ZG1hLXNnLmMgICAgIHwgIDggKysrKy0KPiAgZHJpdmVycy9tZWRpYS92NGwyLWNvcmUvdmlkZW9i dWYyLXZtYWxsb2MuYyAgICB8ICA4ICsrKystCj4gIGRyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL2lv bi9pb24uYyAgICAgICAgICAgICAgfCAgOSArKystLQo+ICBpbmNsdWRlL2xpbnV4L2RtYS1idWYu aCAgICAgICAgICAgICAgICAgICAgICAgIHwgMzQgKysrKysrKysrKysrKysrLS0tLQoKRG9jdW1l bnRhdGlvbi9kbWEtYnVmLXNoYXJpbmcudHh0IG5lZWRzIHVwZGF0aW5nIGFzIGEgcmVzdWx0IG9m IHRoZXNlCmNoYW5nZXMgYnV0IGl0cyBub3QgaW4gdGhlIGRpZmZzdGF0LgoKCj4gIDE0IGZpbGVz IGNoYW5nZWQsIDE0MiBpbnNlcnRpb25zKCspLCA1MCBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0t Z2l0IGEvZHJpdmVycy9kbWEtYnVmL2RtYS1idWYuYyBiL2RyaXZlcnMvZG1hLWJ1Zi9kbWEtYnVm LmMKPiBpbmRleCA1YmUyMjVjMmJhOTguLjZkM2RmM2RkOTMxMCAxMDA2NDQKPiAtLS0gYS9kcml2 ZXJzL2RtYS1idWYvZG1hLWJ1Zi5jCj4gKysrIGIvZHJpdmVycy9kbWEtYnVmL2RtYS1idWYuYwo+ IEBAIC0yNjUsNyArMjY1LDcgQEAgc3RhdGljIGlubGluZSBpbnQgaXNfZG1hX2J1Zl9maWxlKHN0 cnVjdCBmaWxlICpmaWxlKQo+ICB9Cj4gIAo+ICAvKioKPiAtICogZG1hX2J1Zl9leHBvcnRfbmFt ZWQgLSBDcmVhdGVzIGEgbmV3IGRtYV9idWYsIGFuZCBhc3NvY2lhdGVzIGFuIGFub24gZmlsZQo+ ICsgKiBkbWFfYnVmX2V4cG9ydCAtIENyZWF0ZXMgYSBuZXcgZG1hX2J1ZiwgYW5kIGFzc29jaWF0 ZXMgYW4gYW5vbiBmaWxlCj4gICAqIHdpdGggdGhpcyBidWZmZXIsIHNvIGl0IGNhbiBiZSBleHBv cnRlZC4KPiAgICogQWxzbyBjb25uZWN0IHRoZSBhbGxvY2F0b3Igc3BlY2lmaWMgZGF0YSBhbmQg b3BzIHRvIHRoZSBidWZmZXIuCj4gICAqIEFkZGl0aW9uYWxseSwgcHJvdmlkZSBhIG5hbWUgc3Ry aW5nIGZvciBleHBvcnRlcjsgdXNlZnVsIGluIGRlYnVnZ2luZy4KPiBAQCAtMjc3LDMxICsyNzcs MzIgQEAgc3RhdGljIGlubGluZSBpbnQgaXNfZG1hX2J1Zl9maWxlKHN0cnVjdCBmaWxlICpmaWxl KQo+ICAgKiBAZXhwX25hbWU6CVtpbl0JbmFtZSBvZiB0aGUgZXhwb3J0aW5nIG1vZHVsZSAtIHVz ZWZ1bCBmb3IgZGVidWdnaW5nLgo+ICAgKiBAcmVzdjoJW2luXQlyZXNlcnZhdGlvbi1vYmplY3Qs IE5VTEwgdG8gYWxsb2NhdGUgZGVmYXVsdCBvbmUuCj4gICAqCj4gKyAqIEFsbCB0aGUgYWJvdmUg aW5mbyBjb21lcyBmcm9tIHN0cnVjdCBkbWFfYnVmX2V4cG9ydF9pbmZvLgo+ICsgKgoKSSdtIG5v dCBhdCBhbGwgc3VyZSBhYm91dCB0aGlzLiBJdHMgYSBub3ZlbCB0cmljayBidXQgd29uJ3QgdGhp cyBtYWtlCnRoZSBIVE1MIGRvY3MgY29tZSBvdXQgbG9va2luZyBhIGJpdCB3ZWlyZD8gSXMgdGhl cmUgYW55IHByaW9yIGFydCBmb3IKZG91YmxlLWRvY3VtZW50aW5nIHRoZSBzdHJ1Y3R1cmUgbWVt YmVycyBsaWtlIHRoaXM/CgoKRGFuaWVsLgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5m cmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3Rp bmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.thompson@linaro.org (Daniel Thompson) Date: Tue, 03 Feb 2015 09:29:21 +0000 Subject: [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible In-Reply-To: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> References: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> Message-ID: <54D094F1.50404@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 28/01/15 12:54, Sumit Semwal wrote: > At present, dma_buf_export() takes a series of parameters, which > makes it difficult to add any new parameters for exporters, if required. > > Make it simpler by moving all these parameters into a struct, and pass > the struct * as parameter to dma_buf_export(). > > While at it, unite dma_buf_export_named() with dma_buf_export(), and > change all callers accordingly. > > Signed-off-by: Sumit Semwal Sorry, a few more comments. Should have sent these before but at least the are all related only to documentation. Once that is fixed then: Reviewed-by: Daniel Thompson > --- > v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using > {.exp_name = xxx} instead. > > v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default > > drivers/dma-buf/dma-buf.c | 47 +++++++++++++------------- > drivers/gpu/drm/armada/armada_gem.c | 10 ++++-- > drivers/gpu/drm/drm_prime.c | 12 ++++--- > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++-- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 ++++-- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 ++++- > drivers/gpu/drm/tegra/gem.c | 10 ++++-- > drivers/gpu/drm/ttm/ttm_object.c | 9 +++-- > drivers/gpu/drm/udl/udl_dmabuf.c | 9 ++++- > drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 ++++- > drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 ++++- > drivers/media/v4l2-core/videobuf2-vmalloc.c | 8 ++++- > drivers/staging/android/ion/ion.c | 9 +++-- > include/linux/dma-buf.h | 34 +++++++++++++++---- Documentation/dma-buf-sharing.txt needs updating as a result of these changes but its not in the diffstat. > 14 files changed, 142 insertions(+), 50 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 5be225c2ba98..6d3df3dd9310 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file) > } > > /** > - * dma_buf_export_named - Creates a new dma_buf, and associates an anon file > + * dma_buf_export - Creates a new dma_buf, and associates an anon file > * with this buffer, so it can be exported. > * Also connect the allocator specific data and ops to the buffer. > * Additionally, provide a name string for exporter; useful in debugging. > @@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file) > * @exp_name: [in] name of the exporting module - useful for debugging. > * @resv: [in] reservation-object, NULL to allocate default one. > * > + * All the above info comes from struct dma_buf_export_info. > + * I'm not at all sure about this. Its a novel trick but won't this make the HTML docs come out looking a bit weird? Is there any prior art for double-documenting the structure members like this? Daniel. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wi0-f173.google.com ([209.85.212.173]:51930 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752427AbbBCJ3X (ORCPT ); Tue, 3 Feb 2015 04:29:23 -0500 Received: by mail-wi0-f173.google.com with SMTP id r20so22716338wiv.0 for ; Tue, 03 Feb 2015 01:29:22 -0800 (PST) Message-ID: <54D094F1.50404@linaro.org> Date: Tue, 03 Feb 2015 09:29:21 +0000 From: Daniel Thompson MIME-Version: 1.0 To: Sumit Semwal , linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-arm-kernel@lists.infradead.org, rmk+kernel@arm.linux.org.uk, airlied@linux.ie, kgene@kernel.org, daniel.vetter@intel.com, thierry.reding@gmail.com, pawel@osciak.com, m.szyprowski@samsung.com, mchehab@osg.samsung.com, gregkh@linuxfoundation.org CC: linaro-kernel@lists.linaro.org, intel-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org Subject: Re: [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible References: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> In-Reply-To: <1422449643-7829-1-git-send-email-sumit.semwal@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org List-ID: On 28/01/15 12:54, Sumit Semwal wrote: > At present, dma_buf_export() takes a series of parameters, which > makes it difficult to add any new parameters for exporters, if required. > > Make it simpler by moving all these parameters into a struct, and pass > the struct * as parameter to dma_buf_export(). > > While at it, unite dma_buf_export_named() with dma_buf_export(), and > change all callers accordingly. > > Signed-off-by: Sumit Semwal Sorry, a few more comments. Should have sent these before but at least the are all related only to documentation. Once that is fixed then: Reviewed-by: Daniel Thompson > --- > v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using > {.exp_name = xxx} instead. > > v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default > > drivers/dma-buf/dma-buf.c | 47 +++++++++++++------------- > drivers/gpu/drm/armada/armada_gem.c | 10 ++++-- > drivers/gpu/drm/drm_prime.c | 12 ++++--- > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++-- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 ++++-- > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 ++++- > drivers/gpu/drm/tegra/gem.c | 10 ++++-- > drivers/gpu/drm/ttm/ttm_object.c | 9 +++-- > drivers/gpu/drm/udl/udl_dmabuf.c | 9 ++++- > drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 ++++- > drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 ++++- > drivers/media/v4l2-core/videobuf2-vmalloc.c | 8 ++++- > drivers/staging/android/ion/ion.c | 9 +++-- > include/linux/dma-buf.h | 34 +++++++++++++++---- Documentation/dma-buf-sharing.txt needs updating as a result of these changes but its not in the diffstat. > 14 files changed, 142 insertions(+), 50 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 5be225c2ba98..6d3df3dd9310 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file) > } > > /** > - * dma_buf_export_named - Creates a new dma_buf, and associates an anon file > + * dma_buf_export - Creates a new dma_buf, and associates an anon file > * with this buffer, so it can be exported. > * Also connect the allocator specific data and ops to the buffer. > * Additionally, provide a name string for exporter; useful in debugging. > @@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file) > * @exp_name: [in] name of the exporting module - useful for debugging. > * @resv: [in] reservation-object, NULL to allocate default one. > * > + * All the above info comes from struct dma_buf_export_info. > + * I'm not at all sure about this. Its a novel trick but won't this make the HTML docs come out looking a bit weird? Is there any prior art for double-documenting the structure members like this? Daniel.